From 4357ac588b5dcea15bd36e3295f74204e61a5619 Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Sat, 5 Mar 2022 23:18:38 +0200 Subject: [PATCH 1/3] Refactor cmd/ to get rid of most direct os package access This makes everything much more easily testable! --- cmd/archive.go | 30 ++- cmd/archive_test.go | 25 ++- cmd/cloud.go | 109 +++++----- cmd/common.go | 10 +- cmd/config.go | 45 ++-- cmd/config_consolidation_test.go | 122 +++-------- cmd/convert.go | 20 +- cmd/convert_test.go | 157 ++++++-------- cmd/inspect.go | 27 ++- cmd/login_cloud.go | 25 +-- cmd/login_influxdb.go | 14 +- cmd/outputs.go | 22 +- cmd/pause.go | 10 +- cmd/resume.go | 10 +- cmd/root.go | 359 +++++++++++++++++++------------ cmd/root_test.go | 73 +++++++ cmd/run.go | 57 ++--- cmd/run_test.go | 159 +++++--------- cmd/runtime_options.go | 17 -- cmd/scale.go | 9 +- cmd/stats.go | 10 +- cmd/status.go | 10 +- cmd/ui.go | 121 +++++------ cmd/ui_test.go | 2 +- cmd/version.go | 6 +- loader/filesystems.go | 3 +- log/file.go | 9 +- log/file_test.go | 13 +- 28 files changed, 723 insertions(+), 751 deletions(-) create mode 100644 cmd/root_test.go diff --git a/cmd/archive.go b/cmd/archive.go index d39a6780ed1..ae60d9d0cda 100644 --- a/cmd/archive.go +++ b/cmd/archive.go @@ -21,10 +21,6 @@ package cmd import ( - "os" - - "github.com/sirupsen/logrus" - "github.com/spf13/afero" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -33,7 +29,8 @@ import ( "go.k6.io/k6/lib/metrics" ) -func getArchiveCmd(logger *logrus.Logger, globalFlags *commandFlags) *cobra.Command { +func getArchiveCmd(globalState *globalState) *cobra.Command { // nolint: funlen + archiveOut := "archive.tar" // archiveCmd represents the archive command archiveCmd := &cobra.Command{ Use: "archive", @@ -49,19 +46,22 @@ An archive is a fully self-contained test run, and can be executed identically e k6 run myarchive.tar`[1:], Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - src, filesystems, err := readSource(args[0], logger) + src, filesystems, err := readSource(globalState, args[0]) if err != nil { return err } - runtimeOptions, err := getRuntimeOptions(cmd.Flags(), buildEnvMap(os.Environ())) + runtimeOptions, err := getRuntimeOptions(cmd.Flags(), globalState.envVars) if err != nil { return err } registry := metrics.NewRegistry() builtinMetrics := metrics.RegisterBuiltinMetrics(registry) - r, err := newRunner(logger, src, globalFlags.runType, filesystems, runtimeOptions, builtinMetrics, registry) + r, err := newRunner( + globalState.logger, src, globalState.flags.runType, + filesystems, runtimeOptions, builtinMetrics, registry, + ) if err != nil { return err } @@ -70,9 +70,7 @@ An archive is a fully self-contained test run, and can be executed identically e if err != nil { return err } - conf, err := getConsolidatedConfig( - afero.NewOsFs(), Config{Options: cliOpts}, r.GetOptions(), buildEnvMap(os.Environ()), globalFlags, - ) + conf, err := getConsolidatedConfig(globalState, Config{Options: cliOpts}, r.GetOptions()) if err != nil { return err } @@ -89,7 +87,7 @@ An archive is a fully self-contained test run, and can be executed identically e } } - _, err = deriveAndValidateConfig(conf, r.IsExecutable, logger) + _, err = deriveAndValidateConfig(conf, r.IsExecutable, globalState.logger) if err != nil { return err } @@ -101,7 +99,7 @@ An archive is a fully self-contained test run, and can be executed identically e // Archive. arc := r.MakeArchive() - f, err := os.Create(globalFlags.archiveOut) + f, err := globalState.fs.Create(archiveOut) if err != nil { return err } @@ -115,16 +113,16 @@ An archive is a fully self-contained test run, and can be executed identically e } archiveCmd.Flags().SortFlags = false - archiveCmd.Flags().AddFlagSet(archiveCmdFlagSet(globalFlags)) + archiveCmd.Flags().AddFlagSet(archiveCmdFlagSet(&archiveOut)) return archiveCmd } -func archiveCmdFlagSet(globalFlags *commandFlags) *pflag.FlagSet { +func archiveCmdFlagSet(archiveOut *string) *pflag.FlagSet { flags := pflag.NewFlagSet("", pflag.ContinueOnError) flags.SortFlags = false flags.AddFlagSet(optionFlagSet()) flags.AddFlagSet(runtimeOptionFlagSet(false)) - flags.StringVarP(&globalFlags.archiveOut, "archive-out", "O", globalFlags.archiveOut, "archive output filename") + flags.StringVarP(archiveOut, "archive-out", "O", *archiveOut, "archive output filename") return flags } diff --git a/cmd/archive_test.go b/cmd/archive_test.go index 3e1d36d6e18..09f71abae70 100644 --- a/cmd/archive_test.go +++ b/cmd/archive_test.go @@ -1,14 +1,15 @@ package cmd import ( + "io/ioutil" "path/filepath" "testing" + "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.k6.io/k6/errext" "go.k6.io/k6/errext/exitcodes" - "go.k6.io/k6/lib/testutils" ) func TestArchiveThresholds(t *testing.T) { @@ -40,20 +41,17 @@ func TestArchiveThresholds(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { t.Parallel() - tmpPath := filepath.Join(t.TempDir(), "archive.tar") - - cmd := getArchiveCmd(testutils.NewLogger(t), newCommandFlags()) - filename, err := filepath.Abs(testCase.testFilename) + testScript, err := ioutil.ReadFile(testCase.testFilename) require.NoError(t, err) - args := []string{filename, "--archive-out", tmpPath} + + testState := newGlobalTestState(t) + require.NoError(t, afero.WriteFile(testState.fs, filepath.Join(testState.cwd, testCase.testFilename), testScript, 0o644)) + testState.args = []string{"k6", "archive", testCase.testFilename} if testCase.noThresholds { - args = append(args, "--no-thresholds") + testState.args = append(testState.args, "--no-thresholds") } - cmd.SetArgs(args) - wantExitCode := exitcodes.InvalidConfig - var gotErrExt errext.HasExitCode - gotErr := cmd.Execute() + gotErr := newRootCommand(testState.globalState).cmd.Execute() assert.Equal(t, testCase.wantErr, @@ -62,9 +60,10 @@ func TestArchiveThresholds(t *testing.T) { ) if testCase.wantErr { + var gotErrExt errext.HasExitCode require.ErrorAs(t, gotErr, &gotErrExt) - assert.Equalf(t, wantExitCode, gotErrExt.ExitCode(), - "status code must be %d", wantExitCode, + assert.Equalf(t, exitcodes.InvalidConfig, gotErrExt.ExitCode(), + "status code must be %d", exitcodes.InvalidConfig, ) } }) diff --git a/cmd/cloud.go b/cmd/cloud.go index 6eb7366024b..06f3acdf74a 100644 --- a/cmd/cloud.go +++ b/cmd/cloud.go @@ -27,7 +27,6 @@ import ( "errors" "fmt" "os" - "os/signal" "path/filepath" "strconv" "sync" @@ -35,8 +34,6 @@ import ( "time" "github.com/fatih/color" - "github.com/sirupsen/logrus" - "github.com/spf13/afero" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -50,7 +47,10 @@ import ( ) //nolint:funlen,gocognit,gocyclo,cyclop -func getCloudCmd(ctx context.Context, logger *logrus.Logger, globalFlags *commandFlags) *cobra.Command { +func getCloudCmd(globalState *globalState) *cobra.Command { + showCloudLogs := true + exitOnRunning := false + cloudCmd := &cobra.Command{ Use: "cloud", Short: "Run a test on the cloud", @@ -60,56 +60,71 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth Example: ` 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 { - // we specifically first parse it and return an error if it has bad value and then check if - // we are going to set it ... so we always parse it instead of it breaking the command if - // the cli flag is removed - if showCloudLogsEnv, ok := os.LookupEnv("K6_SHOW_CLOUD_LOGS"); ok { + PreRunE: func(cmd *cobra.Command, args []string) error { + // TODO: refactor (https://github.com/loadimpact/k6/issues/883) + // + // We deliberately parse the env variables, to validate for wrong + // values, even if we don't subsequently use them (if the respective + // CLI flag was specified, since it has a higher priority). + if showCloudLogsEnv, ok := globalState.envVars["K6_SHOW_CLOUD_LOGS"]; ok { showCloudLogsValue, err := strconv.ParseBool(showCloudLogsEnv) if err != nil { return fmt.Errorf("parsing K6_SHOW_CLOUD_LOGS returned an error: %w", err) } if !cmd.Flags().Changed("show-logs") { - globalFlags.showCloudLogs = showCloudLogsValue + showCloudLogs = showCloudLogsValue } } + + if exitOnRunningEnv, ok := globalState.envVars["K6_EXIT_ON_RUNNING"]; ok { + exitOnRunningValue, err := strconv.ParseBool(exitOnRunningEnv) + if err != nil { + return fmt.Errorf("parsing K6_EXIT_ON_RUNNING returned an error: %w", err) + } + if !cmd.Flags().Changed("exit-on-running") { + exitOnRunning = exitOnRunningValue + } + } + + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { // TODO: disable in quiet mode? - _, _ = fmt.Fprintf(globalFlags.stdout, "\n%s\n\n", getBanner(globalFlags.noColor || !globalFlags.stdoutTTY)) + _, _ = fmt.Fprintf(globalState.stdOut, "\n%s\n\n", getBanner(globalState.flags.noColor || !globalState.stdOut.isTTY)) + logger := globalState.logger progressBar := pb.New( pb.WithConstLeft("Init"), pb.WithConstProgress(0, "Parsing script"), ) - printBar(progressBar, globalFlags) + printBar(globalState, progressBar) // Runner filename := args[0] - src, filesystems, err := readSource(filename, logger) + src, filesystems, err := readSource(globalState, filename) if err != nil { return err } - osEnvironment := buildEnvMap(os.Environ()) - runtimeOptions, err := getRuntimeOptions(cmd.Flags(), osEnvironment) + runtimeOptions, err := getRuntimeOptions(cmd.Flags(), globalState.envVars) if err != nil { return err } - modifyAndPrintBar(progressBar, globalFlags, pb.WithConstProgress(0, "Getting script options")) + modifyAndPrintBar(globalState, progressBar, pb.WithConstProgress(0, "Getting script options")) registry := metrics.NewRegistry() builtinMetrics := metrics.RegisterBuiltinMetrics(registry) - r, err := newRunner(logger, src, globalFlags.runType, filesystems, runtimeOptions, builtinMetrics, registry) + r, err := newRunner(logger, src, globalState.flags.runType, filesystems, runtimeOptions, builtinMetrics, registry) if err != nil { return err } - modifyAndPrintBar(progressBar, globalFlags, pb.WithConstProgress(0, "Consolidating options")) + modifyAndPrintBar(globalState, progressBar, pb.WithConstProgress(0, "Consolidating options")) cliOpts, err := getOptions(cmd.Flags()) if err != nil { return err } - conf, err := getConsolidatedConfig( - afero.NewOsFs(), Config{Options: cliOpts}, r.GetOptions(), buildEnvMap(os.Environ()), globalFlags) + conf, err := getConsolidatedConfig(globalState, Config{Options: cliOpts}, r.GetOptions()) if err != nil { return err } @@ -140,7 +155,7 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth return err } - modifyAndPrintBar(progressBar, globalFlags, pb.WithConstProgress(0, "Building the archive")) + modifyAndPrintBar(globalState, progressBar, pb.WithConstProgress(0, "Building the archive")) arc := r.MakeArchive() // TODO: Fix this // We reuse cloud.Config for parsing options.ext.loadimpact, but this probably shouldn't be @@ -160,7 +175,7 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth // Cloud config cloudConfig, err := cloudapi.GetConsolidatedConfig( - derivedConf.Collectors["cloud"], osEnvironment, "", arc.Options.External) + derivedConf.Collectors["cloud"], globalState.envVars, "", arc.Options.External) if err != nil { return err } @@ -194,27 +209,27 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth name = filepath.Base(filename) } - globalCtx, globalCancel := context.WithCancel(ctx) + globalCtx, globalCancel := context.WithCancel(globalState.ctx) defer globalCancel() // Start cloud test run - modifyAndPrintBar(progressBar, globalFlags, pb.WithConstProgress(0, "Validating script options")) + modifyAndPrintBar(globalState, progressBar, pb.WithConstProgress(0, "Validating script options")) client := cloudapi.NewClient( logger, cloudConfig.Token.String, cloudConfig.Host.String, consts.Version, cloudConfig.Timeout.TimeDuration()) if err = client.ValidateOptions(arc.Options); err != nil { return err } - modifyAndPrintBar(progressBar, globalFlags, pb.WithConstProgress(0, "Uploading archive")) + modifyAndPrintBar(globalState, progressBar, pb.WithConstProgress(0, "Uploading archive")) refID, err := client.StartCloudTestRun(name, cloudConfig.ProjectID.Int64, arc) if err != nil { return err } // Trap Interrupts, SIGINTs and SIGTERMs. - sigC := make(chan os.Signal, 1) - signal.Notify(sigC, os.Interrupt, syscall.SIGINT, syscall.SIGTERM) - defer signal.Stop(sigC) + sigC := make(chan os.Signal, 2) + globalState.signalNotify(sigC, os.Interrupt, syscall.SIGINT, syscall.SIGTERM) + defer globalState.signalStop(sigC) go func() { sig := <-sigC logger.WithField("sig", sig).Print("Stopping cloud test run in response to signal...") @@ -241,15 +256,12 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth testURL := cloudapi.URLForResults(refID, cloudConfig) executionPlan := derivedConf.Scenarios.GetFullExecutionRequirements(et) printExecutionDescription( - "cloud", filename, testURL, derivedConf, et, - executionPlan, nil, globalFlags.noColor || !globalFlags.stdoutTTY, globalFlags, + globalState, "cloud", filename, testURL, derivedConf, et, executionPlan, nil, ) modifyAndPrintBar( - progressBar, - globalFlags, - pb.WithConstLeft("Run "), - pb.WithConstProgress(0, "Initializing the cloud test"), + globalState, progressBar, + pb.WithConstLeft("Run "), pb.WithConstProgress(0, "Initializing the cloud test"), ) progressCtx, progressCancel := context.WithCancel(globalCtx) @@ -258,7 +270,7 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth defer progressBarWG.Wait() defer progressCancel() go func() { - showProgress(progressCtx, []*pb.ProgressBar{progressBar}, logger, globalFlags) + showProgress(progressCtx, globalState, []*pb.ProgressBar{progressBar}, logger) progressBarWG.Done() }() @@ -300,7 +312,7 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth ) ticker := time.NewTicker(time.Millisecond * 2000) - if globalFlags.showCloudLogs { + if showCloudLogs { go func() { logger.Debug("Connecting to cloud logs server...") if err := cloudConfig.StreamLogsToLogger(globalCtx, logger, refID, 0); err != nil { @@ -321,7 +333,7 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth testProgressLock.Unlock() if (newTestProgress.RunStatus > lib.RunStatusRunning) || - (globalFlags.exitOnRunning && newTestProgress.RunStatus == lib.RunStatusRunning) { + (exitOnRunning && newTestProgress.RunStatus == lib.RunStatusRunning) { globalCancel() break } @@ -332,8 +344,8 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth return errext.WithExitCodeIfNone(errors.New("Test progress error"), exitcodes.CloudFailedToGetProgress) } - valueColor := getColor(globalFlags.noColor || !globalFlags.stdoutTTY, color.FgCyan) - fprintf(globalFlags.stdout, " test status: %s\n", valueColor.Sprint(testProgress.RunStatusText)) + valueColor := getColor(globalState.flags.noColor || !globalState.stdOut.isTTY, color.FgCyan) + fprintf(globalState.stdOut, " test status: %s\n", valueColor.Sprint(testProgress.RunStatusText)) if testProgress.ResultStatus == cloudapi.ResultStatusFailed { // TODO: use different exit codes for failed thresholds vs failed test (e.g. aborted by system/limit) @@ -345,27 +357,20 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth }, } cloudCmd.Flags().SortFlags = false - cloudCmd.Flags().AddFlagSet(cloudCmdFlagSet(globalFlags)) + cloudCmd.Flags().AddFlagSet(cloudCmdFlagSet(&showCloudLogs, &exitOnRunning)) return cloudCmd } -func cloudCmdFlagSet(globalFlags *commandFlags) *pflag.FlagSet { +func cloudCmdFlagSet(showCloudLogs, exitOnRunning *bool) *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(&globalFlags.exitOnRunning, "exit-on-running", globalFlags.exitOnRunning, "exits when test reaches the running status") //nolint:lll - // 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" - - // read the comments above for explanation why this is done this way and what are the problems - flags.BoolVar(&globalFlags.showCloudLogs, "show-logs", globalFlags.showCloudLogs, + // TODO: Figure out a better way to handle the CLI flags + flags.BoolVar(exitOnRunning, "exit-on-running", *exitOnRunning, + "exits when test reaches the running status") + flags.BoolVar(showCloudLogs, "show-logs", *showCloudLogs, "enable showing of logs when a test is executed in the cloud") return flags diff --git a/cmd/common.go b/cmd/common.go index 65657e90afa..8b088a50fea 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -25,9 +25,7 @@ import ( "bytes" "fmt" "io" - "os" - "github.com/sirupsen/logrus" "github.com/spf13/afero" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -91,14 +89,14 @@ func exactArgsWithMsg(n int, msg string) cobra.PositionalArgs { // readSource is a small wrapper around loader.ReadSource returning // result of the load and filesystems map -func readSource(filename string, logger *logrus.Logger) (*loader.SourceData, map[string]afero.Fs, error) { - pwd, err := os.Getwd() +func readSource(globalState *globalState, filename string) (*loader.SourceData, map[string]afero.Fs, error) { + pwd, err := globalState.getwd() if err != nil { return nil, nil, err } - filesystems := loader.CreateFilesystems() - src, err := loader.ReadSource(logger, filename, pwd, filesystems, os.Stdin) + filesystems := loader.CreateFilesystems(globalState.fs) + src, err := loader.ReadSource(globalState.logger, filename, pwd, filesystems, globalState.stdIn) return src, filesystems, err } diff --git a/cmd/config.go b/cmd/config.go index 2dd0462dabc..6a4c8fae5cd 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -109,51 +109,42 @@ func getConfig(flags *pflag.FlagSet) (Config, error) { }, nil } -// 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, globalFlags *commandFlags) (Config, string, error) { - realConfigFilePath := globalFlags.configFilePath - if realConfigFilePath == "" { - // The user didn't specify K6_CONFIG or --config, use the default path - realConfigFilePath = globalFlags.defaultConfigFilePath - } - +// Reads the configuration file from the supplied filesystem and returns it or +// an error. The only situation in which an error won't be returned is if the +// user didn't explicitly specify a config file path and the default config file +// doesn't exist. +func readDiskConfig(globalState *globalState) (Config, error) { // Try to see if the file exists in the supplied filesystem - if _, err := fs.Stat(realConfigFilePath); err != nil { - if os.IsNotExist(err) && globalFlags.configFilePath == "" { + if _, err := globalState.fs.Stat(globalState.flags.configFilePath); err != nil { + if os.IsNotExist(err) && globalState.flags.configFilePath == globalState.defaultFlags.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 + return Config{}, err } - data, err := afero.ReadFile(fs, realConfigFilePath) + data, err := afero.ReadFile(globalState.fs, globalState.flags.configFilePath) if err != nil { - return Config{}, realConfigFilePath, err + return Config{}, err } var conf Config - err = json.Unmarshal(data, &conf) - return conf, realConfigFilePath, err + return conf, json.Unmarshal(data, &conf) } // 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 { +func writeDiskConfig(globalState *globalState, conf Config) error { data, err := json.MarshalIndent(conf, "", " ") if err != nil { return err } - if err := fs.MkdirAll(filepath.Dir(configPath), 0o755); err != nil { + if err := globalState.fs.MkdirAll(filepath.Dir(globalState.flags.configFilePath), 0o755); err != nil { return err } - return afero.WriteFile(fs, configPath, data, 0o644) + return afero.WriteFile(globalState.fs, globalState.flags.configFilePath, data, 0o644) } // Reads configuration variables from the environment. @@ -176,16 +167,14 @@ func readEnvConfig(envMap map[string]string) (Config, error) { // - set some defaults if they weren't previously specified // TODO: add better validation, more explicit default values and improve consistency between formats // TODO: accumulate all errors and differentiate between the layers? -func getConsolidatedConfig( - fs afero.Fs, cliConf Config, runnerOpts lib.Options, envMap map[string]string, globalFlags *commandFlags, -) (conf Config, err error) { +func getConsolidatedConfig(globalState *globalState, cliConf Config, runnerOpts lib.Options) (conf Config, err error) { // TODO: use errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) where it makes sense? - fileConf, _, err := readDiskConfig(fs, globalFlags) + fileConf, err := readDiskConfig(globalState) if err != nil { return conf, err } - envConf, err := readEnvConfig(envMap) + envConf, err := readEnvConfig(globalState.envVars) if err != nil { return conf, err } diff --git a/cmd/config_consolidation_test.go b/cmd/config_consolidation_test.go index 08195ef5fa3..a1dceb5d5dc 100644 --- a/cmd/config_consolidation_test.go +++ b/cmd/config_consolidation_test.go @@ -20,24 +20,18 @@ package cmd import ( - "context" "fmt" - "os" "path/filepath" "testing" "time" - "github.com/sirupsen/logrus" "github.com/spf13/afero" - "github.com/spf13/pflag" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/guregu/null.v3" "go.k6.io/k6/lib" "go.k6.io/k6/lib/executor" - "go.k6.io/k6/lib/testutils" - "go.k6.io/k6/lib/testutils/minirunner" "go.k6.io/k6/lib/types" "go.k6.io/k6/stats" ) @@ -129,26 +123,6 @@ func buildStages(durationsAndVUs ...int64) []executor.Stage { return result } -func mostFlagSets() []flagSetInit { - // TODO: make this unnecessary... currently these are the only commands in which - // getConsolidatedConfig() is used, but they also have differences in their CLI flags :/ - // sigh... compromises... - result := []flagSetInit{} - for i, fsi := range []func(globalFlags *commandFlags) *pflag.FlagSet{runCmdFlagSet, archiveCmdFlagSet, cloudCmdFlagSet} { - i, fsi := i, fsi // go... - // TODO: this still uses os.GetEnv which needs to be removed - // before/along adding tests for those fields - root := newRootCommand(context.Background(), nil, nil) - result = append(result, func() (*pflag.FlagSet, *commandFlags) { - flags := pflag.NewFlagSet(fmt.Sprintf("superContrivedFlags_%d", i), pflag.ContinueOnError) - flags.AddFlagSet(root.rootCmdPersistentFlagSet()) - flags.AddFlagSet(fsi(root.commandFlags)) - return flags, root.commandFlags - }) - } - return result -} - type file struct { filepath, contents string } @@ -161,20 +135,12 @@ func getFS(files []file) afero.Fs { return fs } -type flagSetInit func() (*pflag.FlagSet, *commandFlags) - type opts struct { cli []string env []string runner *lib.Options fs afero.Fs - - // TODO: remove this when the configuration is more reproducible and sane... - // We use a func, because initializing a FlagSet that points to variables - // actually will change those variables to their default values :| In our - // case, this happens only some of the time, for global variables that - // are configurable only via CLI flags, but not environment variables. - cliFlagSetInits []flagSetInit + cmds []string } // exp contains the different events or errors we expect our test case to trigger. @@ -197,17 +163,8 @@ type configConsolidationTestCase struct { func getConfigConsolidationTestCases() []configConsolidationTestCase { defaultConfig := func(jsonConfig string) afero.Fs { - confDir, err := os.UserConfigDir() - if err != nil { - confDir = ".config" - } return getFS([]file{{ - filepath.Join( - confDir, - "loadimpact", - "k6", - defaultConfigFileName, - ), + filepath.Join(".config", "loadimpact", "k6", defaultConfigFileName), // TODO: improve jsonConfig, }}) } @@ -533,61 +490,48 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase { } } -func runTestCase( - t *testing.T, - testCase configConsolidationTestCase, - newFlagSet flagSetInit, -) { - t.Helper() - t.Logf("Test with opts=%#v and exp=%#v\n", testCase.options, testCase.expected) - output := testutils.NewTestOutput(t) - logHook := &testutils.SimpleLogrusHook{ - HookedLevels: []logrus.Level{logrus.WarnLevel}, - } +func runTestCase(t *testing.T, testCase configConsolidationTestCase, subCmd string) { + t.Logf("Test for `k6 %s` with opts=%#v and exp=%#v\n", subCmd, testCase.options, testCase.expected) - logHook.Drain() - logger := logrus.New() - logger.AddHook(logHook) - logger.SetOutput(output) + ts := newGlobalTestState(t) + ts.args = append([]string{"k6", subCmd}, testCase.options.cli...) + ts.envVars = buildEnvMap(testCase.options.env) + if testCase.options.fs != nil { + ts.globalState.fs = testCase.options.fs + } - flagSet, globalFlags := newFlagSet() - flagSet.SetOutput(output) - // flagSet.PrintDefaults() + rootCmd := newRootCommand(ts.globalState) + cmd, args, err := rootCmd.cmd.Find(ts.args[1:]) + require.NoError(t, err) - cliErr := flagSet.Parse(testCase.options.cli) + err = cmd.ParseFlags(args) if testCase.expected.cliParseError { - require.Error(t, cliErr) + require.Error(t, err) return } - require.NoError(t, cliErr) + require.NoError(t, err) + + flagSet := cmd.Flags() // TODO: remove these hacks when we improve the configuration... var cliConf Config if flagSet.Lookup("out") != nil { - cliConf, cliErr = getConfig(flagSet) + cliConf, err = getConfig(flagSet) } else { opts, errOpts := getOptions(flagSet) - cliConf, cliErr = Config{Options: opts}, errOpts + cliConf, err = Config{Options: opts}, errOpts } if testCase.expected.cliReadError { - require.Error(t, cliErr) + require.Error(t, err) return } - require.NoError(t, cliErr) + require.NoError(t, err) - var runnerOpts lib.Options + var opts lib.Options if testCase.options.runner != nil { - runnerOpts = minirunner.MiniRunner{Options: *testCase.options.runner}.GetOptions() - } - // without runner creation, values in runnerOpts will simply be invalid - - if testCase.options.fs == nil { - t.Logf("Creating an empty FS for this test") - testCase.options.fs = afero.NewMemMapFs() // create an empty FS if it wasn't supplied + opts = *testCase.options.runner } - consolidatedConfig, err := getConsolidatedConfig(testCase.options.fs, cliConf, runnerOpts, - // TODO: just make testcase.options.env in map[string]string - buildEnvMap(testCase.options.env), globalFlags) + consolidatedConfig, err := getConsolidatedConfig(ts.globalState, cliConf, opts) if testCase.expected.consolidationError { require.Error(t, err) return @@ -595,14 +539,14 @@ func runTestCase( require.NoError(t, err) derivedConfig := consolidatedConfig - derivedConfig.Options, err = executor.DeriveScenariosFromShortcuts(consolidatedConfig.Options, logger) + derivedConfig.Options, err = executor.DeriveScenariosFromShortcuts(consolidatedConfig.Options, ts.logger) if testCase.expected.derivationError { require.Error(t, err) return } require.NoError(t, err) - if warnings := logHook.Drain(); testCase.expected.logWarning { + if warnings := ts.loggerHook.Drain(); testCase.expected.logWarning { assert.NotEmpty(t, warnings) } else { assert.Empty(t, warnings) @@ -625,17 +569,17 @@ func TestConfigConsolidation(t *testing.T) { for tcNum, testCase := range getConfigConsolidationTestCases() { tcNum, testCase := tcNum, testCase - flagSetInits := testCase.options.cliFlagSetInits - if flagSetInits == nil { // handle the most common case - flagSetInits = mostFlagSets() + subCommands := testCase.options.cmds + if subCommands == nil { // handle the most common case + subCommands = []string{"run", "archive", "cloud"} } - for fsNum, flagSet := range flagSetInits { - fsNum, flagSet := fsNum, flagSet + for fsNum, subCmd := range subCommands { + fsNum, subCmd := fsNum, subCmd t.Run( fmt.Sprintf("TestCase#%d_FlagSet#%d", tcNum, fsNum), func(t *testing.T) { t.Parallel() - runTestCase(t, testCase, flagSet) + runTestCase(t, testCase, subCmd) }, ) } diff --git a/cmd/convert.go b/cmd/convert.go index dd7dcf0acd4..62d3a8ded07 100644 --- a/cmd/convert.go +++ b/cmd/convert.go @@ -23,8 +23,6 @@ package cmd import ( "encoding/json" "io" - "io/ioutil" - "path/filepath" "github.com/spf13/afero" "github.com/spf13/cobra" @@ -35,7 +33,7 @@ import ( ) //nolint:funlen,gocognit -func getConvertCmd(defaultFs afero.Fs, defaultWriter io.Writer) *cobra.Command { +func getConvertCmd(globalState *globalState) *cobra.Command { var ( convertOutput string optionsFilePath string @@ -68,11 +66,7 @@ func getConvertCmd(defaultFs afero.Fs, defaultWriter io.Writer) *cobra.Command { Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { // Parse the HAR file - filePath, err := filepath.Abs(args[0]) - if err != nil { - return err - } - r, err := defaultFs.Open(filePath) + r, err := globalState.fs.Open(args[0]) if err != nil { return err } @@ -88,9 +82,9 @@ func getConvertCmd(defaultFs afero.Fs, defaultWriter io.Writer) *cobra.Command { options := lib.Options{MaxRedirects: null.IntFrom(0)} if optionsFilePath != "" { - optionsFileContents, err := ioutil.ReadFile(optionsFilePath) //nolint:gosec,govet - if err != nil { - return err + optionsFileContents, readErr := afero.ReadFile(globalState.fs, optionsFilePath) + if readErr != nil { + return readErr } var injectedOptions lib.Options if err := json.Unmarshal(optionsFileContents, &injectedOptions); err != nil { @@ -108,11 +102,11 @@ func getConvertCmd(defaultFs afero.Fs, defaultWriter io.Writer) *cobra.Command { // Write script content to stdout or file if convertOutput == "" || convertOutput == "-" { //nolint:nestif - if _, err := io.WriteString(defaultWriter, script); err != nil { + if _, err := io.WriteString(globalState.stdOut, script); err != nil { return err } } else { - f, err := defaultFs.Create(convertOutput) + f, err := globalState.fs.Create(convertOutput) if err != nil { return err } diff --git a/cmd/convert_test.go b/cmd/convert_test.go index 748989ed154..5ec286c0bc0 100644 --- a/cmd/convert_test.go +++ b/cmd/convert_test.go @@ -21,9 +21,7 @@ package cmd import ( - "bytes" "io/ioutil" - "path/filepath" "regexp" "testing" @@ -121,95 +119,70 @@ export default function() { } ` -func TestIntegrationConvertCmd(t *testing.T) { +func TestConvertCmdCorrelate(t *testing.T) { t.Parallel() - t.Run("Correlate", func(t *testing.T) { - t.Parallel() - harFile, err := filepath.Abs("correlate.har") - require.NoError(t, err) - har, err := ioutil.ReadFile("testdata/example.har") - require.NoError(t, err) - - expectedTestPlan, err := ioutil.ReadFile("testdata/example.js") - require.NoError(t, err) - - defaultFs := afero.NewMemMapFs() - - err = afero.WriteFile(defaultFs, harFile, har, 0o644) - require.NoError(t, err) - - buf := &bytes.Buffer{} - - convertCmd := getConvertCmd(defaultFs, buf) - assert.NoError(t, convertCmd.Flags().Set("correlate", "true")) - assert.NoError(t, convertCmd.Flags().Set("no-batch", "true")) - assert.NoError(t, convertCmd.Flags().Set("enable-status-code-checks", "true")) - assert.NoError(t, convertCmd.Flags().Set("return-on-failed-check", "true")) - - err = convertCmd.RunE(convertCmd, []string{harFile}) - - // reset the convertCmd to default flags. There must be a nicer and less error prone way to do this... - assert.NoError(t, convertCmd.Flags().Set("correlate", "false")) - assert.NoError(t, convertCmd.Flags().Set("no-batch", "false")) - assert.NoError(t, convertCmd.Flags().Set("enable-status-code-checks", "false")) - assert.NoError(t, convertCmd.Flags().Set("return-on-failed-check", "false")) - - // Sanitizing to avoid windows problems with carriage returns - re := regexp.MustCompile(`\r`) - expected := re.ReplaceAllString(string(expectedTestPlan), ``) - result := re.ReplaceAllString(buf.String(), ``) - - if assert.NoError(t, err) { - // assert.Equal suppresses the diff it is too big, so we add it as the test error message manually as well. - diff, _ := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{ - A: difflib.SplitLines(expected), - B: difflib.SplitLines(result), - FromFile: "Expected", - FromDate: "", - ToFile: "Actual", - ToDate: "", - Context: 1, - }) - - assert.Equal(t, expected, result, diff) - } - }) - t.Run("Stdout", func(t *testing.T) { - t.Parallel() - harFile, err := filepath.Abs("stdout.har") - require.NoError(t, err) - defaultFs := afero.NewMemMapFs() - err = afero.WriteFile(defaultFs, harFile, []byte(testHAR), 0o644) - assert.NoError(t, err) - - buf := &bytes.Buffer{} - - convertCmd := getConvertCmd(defaultFs, buf) - err = convertCmd.RunE(convertCmd, []string{harFile}) - assert.NoError(t, err) - assert.Equal(t, testHARConvertResult, buf.String()) - }) - t.Run("Output file", func(t *testing.T) { - t.Parallel() - harFile, err := filepath.Abs("output.har") - require.NoError(t, err) - defaultFs := afero.NewMemMapFs() - err = afero.WriteFile(defaultFs, harFile, []byte(testHAR), 0o644) - assert.NoError(t, err) - - convertCmd := getConvertCmd(defaultFs, nil) - err = convertCmd.Flags().Set("output", "/output.js") - defer func() { - err = convertCmd.Flags().Set("output", "") - }() - assert.NoError(t, err) - err = convertCmd.RunE(convertCmd, []string{harFile}) - assert.NoError(t, err) - - output, err := afero.ReadFile(defaultFs, "/output.js") - assert.NoError(t, err) - assert.Equal(t, testHARConvertResult, string(output)) - }) - // TODO: test options injection; right now that's difficult because when there are multiple - // options, they can be emitted in different order in the JSON + har, err := ioutil.ReadFile("testdata/example.har") + require.NoError(t, err) + + expectedTestPlan, err := ioutil.ReadFile("testdata/example.js") + require.NoError(t, err) + + testState := newGlobalTestState(t) + require.NoError(t, afero.WriteFile(testState.fs, "correlate.har", har, 0o644)) + testState.args = []string{ + "k6", "convert", "--output=result.js", "--correlate=true", "--no-batch=true", + "--enable-status-code-checks=true", "--return-on-failed-check=true", "correlate.har", + } + + require.NoError(t, newRootCommand(testState.globalState).cmd.Execute()) + + result, err := afero.ReadFile(testState.fs, "result.js") + require.NoError(t, err) + + // Sanitizing to avoid windows problems with carriage returns + re := regexp.MustCompile(`\r`) + expected := re.ReplaceAllString(string(expectedTestPlan), ``) + resultStr := re.ReplaceAllString(string(result), ``) + + if assert.NoError(t, err) { + // assert.Equal suppresses the diff it is too big, so we add it as the test error message manually as well. + diff, _ := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{ + A: difflib.SplitLines(expected), + B: difflib.SplitLines(resultStr), + FromFile: "Expected", + FromDate: "", + ToFile: "Actual", + ToDate: "", + Context: 1, + }) + + assert.Equal(t, expected, resultStr, diff) + } } + +func TestConvertCmdStdout(t *testing.T) { + t.Parallel() + testState := newGlobalTestState(t) + require.NoError(t, afero.WriteFile(testState.fs, "stdout.har", []byte(testHAR), 0o644)) + testState.args = []string{"k6", "convert", "stdout.har"} + + require.NoError(t, newRootCommand(testState.globalState).cmd.Execute()) + assert.Equal(t, testHARConvertResult, testState.stdOut.String()) +} + +func TestConvertCmdOutputFile(t *testing.T) { + t.Parallel() + + testState := newGlobalTestState(t) + require.NoError(t, afero.WriteFile(testState.fs, "output.har", []byte(testHAR), 0o644)) + testState.args = []string{"k6", "convert", "--output", "result.js", "output.har"} + + require.NoError(t, newRootCommand(testState.globalState).cmd.Execute()) + + output, err := afero.ReadFile(testState.fs, "result.js") + assert.NoError(t, err) + assert.Equal(t, testHARConvertResult, string(output)) +} + +// TODO: test options injection; right now that's difficult because when there are multiple +// options, they can be emitted in different order in the JSON diff --git a/cmd/inspect.go b/cmd/inspect.go index 712aaa878ae..2acc5e5e0c1 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -24,10 +24,7 @@ import ( "bytes" "encoding/json" "fmt" - "os" - "github.com/sirupsen/logrus" - "github.com/spf13/afero" "github.com/spf13/cobra" "go.k6.io/k6/js" @@ -36,7 +33,7 @@ import ( "go.k6.io/k6/lib/types" ) -func getInspectCmd(logger *logrus.Logger, globalFlags *commandFlags) *cobra.Command { +func getInspectCmd(globalState *globalState) *cobra.Command { var addExecReqs bool // inspectCmd represents the inspect command @@ -46,19 +43,19 @@ func getInspectCmd(logger *logrus.Logger, globalFlags *commandFlags) *cobra.Comm Long: `Inspect a script or archive.`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - src, filesystems, err := readSource(args[0], logger) + src, filesystems, err := readSource(globalState, args[0]) if err != nil { return err } - runtimeOptions, err := getRuntimeOptions(cmd.Flags(), buildEnvMap(os.Environ())) + runtimeOptions, err := getRuntimeOptions(cmd.Flags(), globalState.envVars) if err != nil { return err } registry := metrics.NewRegistry() var b *js.Bundle - typ := globalFlags.runType + typ := globalState.flags.runType if typ == "" { typ = detectType(src.Data) } @@ -70,10 +67,10 @@ func getInspectCmd(logger *logrus.Logger, globalFlags *commandFlags) *cobra.Comm if err != nil { return err } - b, err = js.NewBundleFromArchive(logger, arc, runtimeOptions, registry) + b, err = js.NewBundleFromArchive(globalState.logger, arc, runtimeOptions, registry) case typeJS: - b, err = js.NewBundle(logger, src, filesystems, runtimeOptions, registry) + b, err = js.NewBundle(globalState.logger, src, filesystems, runtimeOptions, registry) } if err != nil { return err @@ -83,7 +80,7 @@ func getInspectCmd(logger *logrus.Logger, globalFlags *commandFlags) *cobra.Comm inspectOutput := interface{}(b.Options) if addExecReqs { - inspectOutput, err = addExecRequirements(b, logger, globalFlags) + inspectOutput, err = addExecRequirements(globalState, b) if err != nil { return err } @@ -101,7 +98,8 @@ func getInspectCmd(logger *logrus.Logger, globalFlags *commandFlags) *cobra.Comm inspectCmd.Flags().SortFlags = false inspectCmd.Flags().AddFlagSet(runtimeOptionFlagSet(false)) - inspectCmd.Flags().StringVarP(&globalFlags.runType, "type", "t", globalFlags.runType, "override file `type`, \"js\" or \"archive\"") //nolint:lll + inspectCmd.Flags().StringVarP(&globalState.flags.runType, "type", "t", + globalState.flags.runType, "override file `type`, \"js\" or \"archive\"") inspectCmd.Flags().BoolVar(&addExecReqs, "execution-requirements", false, @@ -110,14 +108,13 @@ func getInspectCmd(logger *logrus.Logger, globalFlags *commandFlags) *cobra.Comm return inspectCmd } -func addExecRequirements(b *js.Bundle, logger *logrus.Logger, globalFlags *commandFlags) (interface{}, error) { - conf, err := getConsolidatedConfig( - afero.NewOsFs(), Config{}, b.Options, buildEnvMap(os.Environ()), globalFlags) +func addExecRequirements(gs *globalState, b *js.Bundle) (interface{}, error) { + conf, err := getConsolidatedConfig(gs, Config{}, b.Options) if err != nil { return nil, err } - conf, err = deriveAndValidateConfig(conf, b.IsExecutable, logger) + conf, err = deriveAndValidateConfig(conf, b.IsExecutable, gs.logger) if err != nil { return nil, err } diff --git a/cmd/login_cloud.go b/cmd/login_cloud.go index 17902524ce7..775286d5ce3 100644 --- a/cmd/login_cloud.go +++ b/cmd/login_cloud.go @@ -23,12 +23,9 @@ package cmd import ( "encoding/json" "errors" - "os" "syscall" "github.com/fatih/color" - "github.com/sirupsen/logrus" - "github.com/spf13/afero" "github.com/spf13/cobra" "golang.org/x/term" "gopkg.in/guregu/null.v3" @@ -39,7 +36,7 @@ import ( ) //nolint:funlen,gocognit -func getLoginCloudCommand(logger logrus.FieldLogger, globalFlags *commandFlags) *cobra.Command { +func getLoginCloudCommand(globalState *globalState) *cobra.Command { // loginCloudCommand represents the 'login cloud' command loginCloudCommand := &cobra.Command{ Use: "cloud", @@ -58,9 +55,7 @@ This will set the default token used when just "k6 run -o cloud" is passed.`, k6 login cloud`[1:], Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - fs := afero.NewOsFs() - - currentDiskConf, configPath, err := readDiskConfig(fs, globalFlags) + currentDiskConf, err := readDiskConfig(globalState) if err != nil { return err } @@ -77,7 +72,7 @@ This will set the default token used when just "k6 run -o cloud" is passed.`, // We want to use this fully consolidated config for things like // host addresses, so users can overwrite them with env vars. consolidatedCurrentConfig, err := cloudapi.GetConsolidatedConfig( - currentJSONConfigRaw, buildEnvMap(os.Environ()), "", nil) + currentJSONConfigRaw, globalState.envVars, "", nil) if err != nil { return err } @@ -91,7 +86,7 @@ This will set the default token used when just "k6 run -o cloud" is passed.`, switch { case reset.Valid: newCloudConf.Token = null.StringFromPtr(nil) - fprintf(globalFlags.stdout, " token reset\n") + fprintf(globalState.stdOut, " token reset\n") case show.Bool: case token.Valid: newCloudConf.Token = token @@ -109,10 +104,10 @@ This will set the default token used when just "k6 run -o cloud" is passed.`, }, } if !term.IsTerminal(int(syscall.Stdin)) { // nolint: unconvert - logger.Warn("Stdin is not a terminal, falling back to plain text input") + globalState.logger.Warn("Stdin is not a terminal, falling back to plain text input") } var vals map[string]string - vals, err = form.Run(os.Stdin, globalFlags.stdout) + vals, err = form.Run(globalState.stdIn, globalState.stdOut) if err != nil { return err } @@ -120,7 +115,7 @@ This will set the default token used when just "k6 run -o cloud" is passed.`, password := vals["Password"] client := cloudapi.NewClient( - logger, + globalState.logger, "", consolidatedCurrentConfig.Host.String, consts.Version, @@ -146,13 +141,13 @@ This will set the default token used when just "k6 run -o cloud" is passed.`, if err != nil { return err } - if err := writeDiskConfig(fs, configPath, currentDiskConf); err != nil { + if err := writeDiskConfig(globalState, currentDiskConf); err != nil { return err } if newCloudConf.Token.Valid { - valueColor := getColor(globalFlags.noColor || !globalFlags.stdoutTTY, color.FgCyan) - fprintf(globalFlags.stdout, " token: %s\n", valueColor.Sprint(newCloudConf.Token.String)) + valueColor := getColor(globalState.flags.noColor || !globalState.stdOut.isTTY, color.FgCyan) + fprintf(globalState.stdOut, " token: %s\n", valueColor.Sprint(newCloudConf.Token.String)) } return nil }, diff --git a/cmd/login_influxdb.go b/cmd/login_influxdb.go index bdf71153854..b5d5544b090 100644 --- a/cmd/login_influxdb.go +++ b/cmd/login_influxdb.go @@ -22,12 +22,9 @@ package cmd import ( "encoding/json" - "os" "syscall" "time" - "github.com/sirupsen/logrus" - "github.com/spf13/afero" "github.com/spf13/cobra" "golang.org/x/term" "gopkg.in/guregu/null.v3" @@ -37,7 +34,7 @@ import ( ) //nolint:funlen -func getLoginInfluxDBCommand(logger logrus.FieldLogger, globalFlags *commandFlags) *cobra.Command { +func getLoginInfluxDBCommand(globalState *globalState) *cobra.Command { // loginInfluxDBCommand represents the 'login influxdb' command loginInfluxDBCommand := &cobra.Command{ Use: "influxdb [uri]", @@ -47,8 +44,7 @@ func getLoginInfluxDBCommand(logger logrus.FieldLogger, globalFlags *commandFlag 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, configPath, err := readDiskConfig(fs, globalFlags) + config, err := readDiskConfig(globalState) if err != nil { return err } @@ -94,9 +90,9 @@ This will set the default server used when just "-o influxdb" is passed.`, }, } if !term.IsTerminal(int(syscall.Stdin)) { // nolint: unconvert - logger.Warn("Stdin is not a terminal, falling back to plain text input") + globalState.logger.Warn("Stdin is not a terminal, falling back to plain text input") } - vals, err := form.Run(os.Stdin, globalFlags.stdout) + vals, err := form.Run(globalState.stdIn, globalState.stdOut) if err != nil { return err } @@ -121,7 +117,7 @@ This will set the default server used when just "-o influxdb" is passed.`, if err != nil { return err } - return writeDiskConfig(fs, configPath, config) + return writeDiskConfig(globalState, config) }, } return loginInfluxDBCommand diff --git a/cmd/outputs.go b/cmd/outputs.go index 6136858c86e..107e61ec546 100644 --- a/cmd/outputs.go +++ b/cmd/outputs.go @@ -26,9 +26,6 @@ import ( "sort" "strings" - "github.com/sirupsen/logrus" - "github.com/spf13/afero" - "go.k6.io/k6/lib" "go.k6.io/k6/loader" "go.k6.io/k6/output" @@ -82,9 +79,8 @@ func getPossibleIDList(constrs map[string]func(output.Params) (output.Output, er } func createOutputs( - outputFullArguments []string, src *loader.SourceData, conf Config, rtOpts lib.RuntimeOptions, - executionPlan []lib.ExecutionStep, osEnvironment map[string]string, logger logrus.FieldLogger, - globalFlags *commandFlags, + gs *globalState, src *loader.SourceData, conf Config, + rtOpts lib.RuntimeOptions, executionPlan []lib.ExecutionStep, ) ([]output.Output, error) { outputConstructors, err := getAllOutputConstructors() if err != nil { @@ -92,18 +88,18 @@ func createOutputs( } baseParams := output.Params{ ScriptPath: src.URL, - Logger: logger, - Environment: osEnvironment, - StdOut: globalFlags.stdout, - StdErr: globalFlags.stderr, - FS: afero.NewOsFs(), + Logger: gs.logger, + Environment: gs.envVars, + StdOut: gs.stdOut, + StdErr: gs.stdErr, + FS: gs.fs, ScriptOptions: conf.Options, RuntimeOptions: rtOpts, ExecutionPlan: executionPlan, } - result := make([]output.Output, 0, len(outputFullArguments)) + result := make([]output.Output, 0, len(conf.Out)) - for _, outputFullArg := range outputFullArguments { + for _, outputFullArg := range conf.Out { outputType, outputArg := parseOutputArgument(outputFullArg) outputConstructor, ok := outputConstructors[outputType] if !ok { diff --git a/cmd/pause.go b/cmd/pause.go index ed3d6d8afc4..bc5629c6e0a 100644 --- a/cmd/pause.go +++ b/cmd/pause.go @@ -21,8 +21,6 @@ package cmd import ( - "context" - "github.com/spf13/cobra" "gopkg.in/guregu/null.v3" @@ -30,7 +28,7 @@ import ( "go.k6.io/k6/api/v1/client" ) -func getPauseCmd(ctx context.Context, globalFlags *commandFlags) *cobra.Command { +func getPauseCmd(globalState *globalState) *cobra.Command { // pauseCmd represents the pause command pauseCmd := &cobra.Command{ Use: "pause", @@ -39,17 +37,17 @@ func getPauseCmd(ctx context.Context, globalFlags *commandFlags) *cobra.Command Use the global --address flag to specify the URL to the API server.`, RunE: func(cmd *cobra.Command, args []string) error { - c, err := client.New(globalFlags.address) + c, err := client.New(globalState.flags.address) if err != nil { return err } - status, err := c.SetStatus(ctx, v1.Status{ + status, err := c.SetStatus(globalState.ctx, v1.Status{ Paused: null.BoolFrom(true), }) if err != nil { return err } - return yamlPrint(globalFlags.stdout, status) + return yamlPrint(globalState.stdOut, status) }, } return pauseCmd diff --git a/cmd/resume.go b/cmd/resume.go index 5f1632d9584..d7737973f14 100644 --- a/cmd/resume.go +++ b/cmd/resume.go @@ -21,8 +21,6 @@ package cmd import ( - "context" - "github.com/spf13/cobra" "gopkg.in/guregu/null.v3" @@ -30,7 +28,7 @@ import ( "go.k6.io/k6/api/v1/client" ) -func getResumeCmd(ctx context.Context, globalFlags *commandFlags) *cobra.Command { +func getResumeCmd(globalState *globalState) *cobra.Command { // resumeCmd represents the resume command resumeCmd := &cobra.Command{ Use: "resume", @@ -39,18 +37,18 @@ func getResumeCmd(ctx context.Context, globalFlags *commandFlags) *cobra.Command Use the global --address flag to specify the URL to the API server.`, RunE: func(cmd *cobra.Command, args []string) error { - c, err := client.New(globalFlags.address) + c, err := client.New(globalState.flags.address) if err != nil { return err } - status, err := c.SetStatus(ctx, v1.Status{ + status, err := c.SetStatus(globalState.ctx, v1.Status{ Paused: null.BoolFrom(false), }) if err != nil { return err } - return yamlPrint(globalFlags.stdout, status) + return yamlPrint(globalState.stdOut, status) }, } return resumeCmd diff --git a/cmd/root.go b/cmd/root.go index ecfdac793a2..97d4fe97e72 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -25,9 +25,11 @@ import ( "context" "errors" "fmt" + "io" "io/ioutil" stdlog "log" "os" + "os/signal" "path/filepath" "strings" "sync" @@ -50,96 +52,194 @@ const ( waitRemoteLoggerTimeout = time.Second * 5 ) -// TODO better name - there are other command flags these are just ... non lib.Options ones :shrug: -type commandFlags struct { - defaultConfigFilePath string - configFilePath string - exitOnRunning bool - showCloudLogs bool - runType string - archiveOut string - quiet bool - noColor bool - address string - outMutex *sync.Mutex - stdoutTTY, stderrTTY bool - stdout, stderr *consoleWriter +// globalFlags contains global config values that apply for all k6 sub-commands. +type globalFlags struct { + configFilePath string + runType string + quiet bool + noColor bool + address string + logOutput string + logFormat string + verbose bool } -func newCommandFlags() *commandFlags { - confDir, err := os.UserConfigDir() - if err != nil { - logrus.WithError(err).Warn("could not get config directory") - confDir = ".config" - } - defaultConfigFilePath := filepath.Join( - confDir, - "loadimpact", - "k6", - defaultConfigFileName, - ) +// globalState contains the globalFlags and accessors for most of the global +// process-external state like CLI arguments, env vars, standard input, output +// and error, etc. In practice, most of it is normally accessed through the `os` +// package from the Go stdlib. +// +// We group them here so we can prevent direct access to them from the rest of +// the k6 codebase. This gives us the ability to mock them and have robust and +// easy-to-write integration-like tests to check the k6 end-to-end behavior in +// any simulated conditions. +// +// `newGlobalState()` returns a globalState object with the real `os` +// parameters, while `newGlobalTestState()` can be used in tests to create +// simulated environments. +type globalState struct { + ctx context.Context + + fs afero.Fs + getwd func() (string, error) + args []string + envVars map[string]string + + defaultFlags, flags globalFlags + + outMutex *sync.Mutex + stdOut, stdErr *consoleWriter + stdIn *os.File + + signalNotify func(chan<- os.Signal, ...os.Signal) + signalStop func(chan<- os.Signal) + + // TODO: add os.Exit()? + logger *logrus.Logger + fallbackLogger logrus.FieldLogger +} + +// Ideally, this should be the only function in the whole codebase where we use +// global variables and functions from the os package. Anywhere else, things +// like os.Stdout, os.Stderr, os.Stdin, os.Getenv(), etc. should be removed and +// the respective properties of globalState used instead. +func newGlobalState(ctx context.Context) *globalState { isDumbTerm := os.Getenv("TERM") == "dumb" stdoutTTY := !isDumbTerm && (isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsCygwinTerminal(os.Stdout.Fd())) stderrTTY := !isDumbTerm && (isatty.IsTerminal(os.Stderr.Fd()) || isatty.IsCygwinTerminal(os.Stderr.Fd())) outMutex := &sync.Mutex{} - return &commandFlags{ - defaultConfigFilePath: defaultConfigFilePath, // Updated with the user's config folder in the init() function below - configFilePath: os.Getenv("K6_CONFIG"), // Overridden by `-c`/`--config` flag! - exitOnRunning: os.Getenv("K6_EXIT_ON_RUNNING") != "", - showCloudLogs: true, - runType: os.Getenv("K6_TYPE"), - archiveOut: "archive.tar", - outMutex: outMutex, - stdoutTTY: stdoutTTY, - stderrTTY: stderrTTY, - stdout: &consoleWriter{colorable.NewColorableStdout(), stdoutTTY, outMutex, nil}, - stderr: &consoleWriter{colorable.NewColorableStderr(), stderrTTY, outMutex, nil}, + stdOut := &consoleWriter{os.Stdout, colorable.NewColorable(os.Stdout), stdoutTTY, outMutex, nil} + stdErr := &consoleWriter{os.Stderr, colorable.NewColorable(os.Stderr), stderrTTY, outMutex, nil} + + logger := getDefaultLogger(stdErr) + + confDir, err := os.UserConfigDir() + if err != nil { + logger.WithError(err).Warn("could not get config directory") + confDir = ".config" + } + + envVars := buildEnvMap(os.Environ()) + defaultFlags := getDefaultFlags(confDir) + + return &globalState{ + ctx: ctx, + fs: afero.NewOsFs(), + getwd: os.Getwd, + args: append(make([]string, 0, len(os.Args)), os.Args...), // copy + envVars: envVars, + defaultFlags: defaultFlags, + flags: getFlags(defaultFlags, envVars), + outMutex: outMutex, + stdOut: stdOut, + stdErr: stdErr, + stdIn: os.Stdin, + signalNotify: signal.Notify, + signalStop: signal.Stop, + logger: logger, + fallbackLogger: getDefaultLogger(stdErr), // we may modify the other one + } +} + +func getDefaultLogger(out io.Writer) *logrus.Logger { + return &logrus.Logger{ + Out: out, + Formatter: new(logrus.TextFormatter), + Hooks: make(logrus.LevelHooks), + Level: logrus.InfoLevel, + } +} + +func getDefaultFlags(homeFolder string) globalFlags { + return globalFlags{ + address: "localhost:6565", + configFilePath: filepath.Join(homeFolder, "loadimpact", "k6", defaultConfigFileName), + logOutput: "stderr", + } +} + +func getFlags(defaultFlags globalFlags, env map[string]string) globalFlags { + result := defaultFlags + + // TODO: add env vars for the rest of the values (after adjusting + // rootCmdPersistentFlagSet(), of course) + + if val, ok := env["K6_CONFIG"]; ok { + result.configFilePath = val + } + if val, ok := env["K6_TYPE"]; ok { + result.runType = val + } + if val, ok := env["K6_LOG_OUTPUT"]; ok { + result.logOutput = val + } + if val, ok := env["K6_LOG_FORMAT"]; ok { + result.logFormat = val } + return result +} + +func parseEnvKeyValue(kv string) (string, string) { + if idx := strings.IndexRune(kv, '='); idx != -1 { + return kv[:idx], kv[idx+1:] + } + return kv, "" +} + +func buildEnvMap(environ []string) map[string]string { + env := make(map[string]string, len(environ)) + for _, kv := range environ { + k, v := parseEnvKeyValue(kv) + env[k] = v + } + return env } // This is to keep all fields needed for the main/root k6 command type rootCommand struct { - ctx context.Context - logger *logrus.Logger - fallbackLogger logrus.FieldLogger + globalState *globalState + cmd *cobra.Command loggerStopped <-chan struct{} - logOutput string - logFmt string loggerIsRemote bool - verbose bool - commandFlags *commandFlags } -func newRootCommand(ctx context.Context, logger *logrus.Logger, fallbackLogger logrus.FieldLogger) *rootCommand { +func newRootCommand(gs *globalState) *rootCommand { c := &rootCommand{ - ctx: ctx, - logger: logger, - fallbackLogger: fallbackLogger, - commandFlags: newCommandFlags(), + globalState: gs, } // the base command when called without any subcommands. - c.cmd = &cobra.Command{ + rootCmd := &cobra.Command{ Use: "k6", Short: "a next-generation load generator", - Long: "\n" + getBanner(c.commandFlags.noColor || !c.commandFlags.stdoutTTY), + Long: "\n" + getBanner(c.globalState.flags.noColor || !c.globalState.stdOut.isTTY), SilenceUsage: true, SilenceErrors: true, PersistentPreRunE: c.persistentPreRunE, } - c.cmd.PersistentFlags().AddFlagSet(c.rootCmdPersistentFlagSet()) + loginCmd := getLoginCmd() + loginCmd.AddCommand( + getLoginCloudCommand(gs), + getLoginInfluxDBCommand(gs), + ) + rootCmd.AddCommand( + getArchiveCmd(gs), getCloudCmd(gs), getConvertCmd(gs), getInspectCmd(gs), + loginCmd, getPauseCmd(gs), getResumeCmd(gs), getScaleCmd(gs), getRunCmd(gs), + getStatsCmd(gs), getStatusCmd(gs), getVersionCmd(gs), + ) + + rootCmd.PersistentFlags().AddFlagSet(rootCmdPersistentFlagSet(gs)) + rootCmd.SetArgs(gs.args[1:]) + c.cmd = rootCmd + return c } func (c *rootCommand) persistentPreRunE(cmd *cobra.Command, args []string) error { var err error - if !cmd.Flags().Changed("log-output") { - if envLogOutput, ok := os.LookupEnv("K6_LOG_OUTPUT"); ok { - c.logOutput = envLogOutput - } - } + c.loggerStopped, err = c.setupLoggers() if err != nil { return err @@ -150,8 +250,8 @@ func (c *rootCommand) persistentPreRunE(cmd *cobra.Command, args []string) error c.loggerIsRemote = true } - stdlog.SetOutput(c.logger.Writer()) - c.logger.Debugf("k6 version: v%s", consts.FullVersion()) + stdlog.SetOutput(c.globalState.logger.Writer()) + c.globalState.logger.Debugf("k6 version: v%s", consts.FullVersion()) return nil } @@ -160,43 +260,12 @@ func (c *rootCommand) persistentPreRunE(cmd *cobra.Command, args []string) error func Execute() { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - logger := &logrus.Logger{ - Out: os.Stderr, - Formatter: new(logrus.TextFormatter), - Hooks: make(logrus.LevelHooks), - Level: logrus.InfoLevel, - } - var fallbackLogger logrus.FieldLogger = &logrus.Logger{ - Out: os.Stderr, - Formatter: new(logrus.TextFormatter), - Hooks: make(logrus.LevelHooks), - Level: logrus.InfoLevel, - } + globalState := newGlobalState(ctx) - c := newRootCommand(ctx, logger, fallbackLogger) + rootCmd := newRootCommand(globalState) - loginCmd := getLoginCmd() - loginCmd.AddCommand( - getLoginCloudCommand(logger, c.commandFlags), - getLoginInfluxDBCommand(logger, c.commandFlags), - ) - c.cmd.AddCommand( - getArchiveCmd(logger, c.commandFlags), - getCloudCmd(ctx, logger, c.commandFlags), - getConvertCmd(afero.NewOsFs(), c.commandFlags.stdout), - getInspectCmd(logger, c.commandFlags), - loginCmd, - getPauseCmd(ctx, c.commandFlags), - getResumeCmd(ctx, c.commandFlags), - getScaleCmd(ctx, c.commandFlags), - getRunCmd(ctx, logger, c.commandFlags), - getStatsCmd(ctx, c.commandFlags), - getStatusCmd(ctx, c.commandFlags), - getVersionCmd(), - ) - - if err := c.cmd.Execute(); err != nil { + if err := rootCmd.cmd.Execute(); err != nil { exitCode := -1 var ecerr errext.HasExitCode if errors.As(err, &ecerr) { @@ -215,18 +284,18 @@ func Execute() { fields["hint"] = herr.Hint() } - logger.WithFields(fields).Error(errText) - if c.loggerIsRemote { - fallbackLogger.WithFields(fields).Error(errText) + globalState.logger.WithFields(fields).Error(errText) + if rootCmd.loggerIsRemote { + globalState.fallbackLogger.WithFields(fields).Error(errText) cancel() - c.waitRemoteLogger() + rootCmd.waitRemoteLogger() } os.Exit(exitCode) //nolint:gocritic } cancel() - c.waitRemoteLogger() + rootCmd.waitRemoteLogger() } func (c *rootCommand) waitRemoteLogger() { @@ -234,28 +303,48 @@ func (c *rootCommand) waitRemoteLogger() { select { case <-c.loggerStopped: case <-time.After(waitRemoteLoggerTimeout): - c.fallbackLogger.Error("Remote logger didn't stop in %s", waitRemoteLoggerTimeout) + c.globalState.fallbackLogger.Errorf("Remote logger didn't stop in %s", waitRemoteLoggerTimeout) } } } -func (c *rootCommand) rootCmdPersistentFlagSet() *pflag.FlagSet { +func rootCmdPersistentFlagSet(gs *globalState) *pflag.FlagSet { flags := pflag.NewFlagSet("", pflag.ContinueOnError) - // TODO: figure out a better way to handle the CLI flags - global variables are not very testable... :/ - flags.BoolVarP(&c.verbose, "verbose", "v", false, "enable verbose logging") - flags.BoolVarP(&c.commandFlags.quiet, "quiet", "q", false, "disable progress updates") - flags.BoolVar(&c.commandFlags.noColor, "no-color", false, "disable colored output") - flags.StringVar(&c.logOutput, "log-output", "stderr", + // TODO: refactor this config, the default value management with pflag is + // simply terrible... :/ + // + // We need to use `gs.flags.` both as the destination and as + // the value here, since the config values could have already been set by + // their respective environment variables. However, we then also have to + // explicitly set the DefValue to the respective default value from + // `gs.defaultFlags.`, so that the `k6 --help` message is + // not messed up... + + flags.StringVar(&gs.flags.logOutput, "log-output", gs.flags.logOutput, "change the output for k6 logs, possible values are stderr,stdout,none,loki[=host:port],file[=./path.fileformat]") - flags.StringVar(&c.logFmt, "logformat", "", "log output format") // TODO rename to log-format and warn on old usage - flags.StringVarP(&c.commandFlags.address, "address", "a", "localhost:6565", "address for the api server") + flags.Lookup("log-output").DefValue = gs.defaultFlags.logOutput + + flags.StringVar(&gs.flags.logFormat, "logformat", gs.flags.logFormat, "log output format") + oldLogFormat := flags.Lookup("logformat") + oldLogFormat.Hidden = true + oldLogFormat.Deprecated = "log-format" + oldLogFormat.DefValue = gs.defaultFlags.logFormat + flags.StringVar(&gs.flags.logFormat, "log-format", gs.flags.logFormat, "log output format") + flags.Lookup("log-format").DefValue = gs.defaultFlags.logFormat - // TODO: Fix... This default value needed, so both CLI flags and environment variables work - flags.StringVarP(&c.commandFlags.configFilePath, "config", "c", c.commandFlags.configFilePath, "JSON config file") + flags.StringVarP(&gs.flags.configFilePath, "config", "c", gs.flags.configFilePath, "JSON config file") // And we also need to explicitly set the default value for the usage message here, so things // like `K6_CONFIG="blah" k6 run -h` don't produce a weird usage message - flags.Lookup("config").DefValue = c.commandFlags.defaultConfigFilePath + flags.Lookup("config").DefValue = gs.defaultFlags.configFilePath must(cobra.MarkFlagFilename(flags, "config")) + + // TODO: support configuring these through environment variables as well? + // either with croconf or through the hack above... + flags.BoolVarP(&gs.flags.verbose, "verbose", "v", gs.defaultFlags.verbose, "enable verbose logging") + flags.BoolVarP(&gs.flags.quiet, "quiet", "q", gs.defaultFlags.quiet, "disable progress updates") + flags.BoolVar(&gs.flags.noColor, "no-color", gs.defaultFlags.noColor, "disable colored output") + flags.StringVarP(&gs.flags.address, "address", "a", gs.defaultFlags.address, "address for the REST API server") + return flags } @@ -274,55 +363,57 @@ func (c *rootCommand) setupLoggers() (<-chan struct{}, error) { ch := make(chan struct{}) close(ch) - if c.verbose { - c.logger.SetLevel(logrus.DebugLevel) + if c.globalState.flags.verbose { + c.globalState.logger.SetLevel(logrus.DebugLevel) } loggerForceColors := false // disable color by default - switch line := c.logOutput; { + switch line := c.globalState.flags.logOutput; { case line == "stderr": - loggerForceColors = !c.commandFlags.noColor && c.commandFlags.stderrTTY - c.logger.SetOutput(c.commandFlags.stderr) + loggerForceColors = !c.globalState.flags.noColor && c.globalState.stdErr.isTTY + c.globalState.logger.SetOutput(c.globalState.stdErr) case line == "stdout": - loggerForceColors = !c.commandFlags.noColor && c.commandFlags.stdoutTTY - c.logger.SetOutput(c.commandFlags.stdout) + loggerForceColors = !c.globalState.flags.noColor && c.globalState.stdOut.isTTY + c.globalState.logger.SetOutput(c.globalState.stdOut) case line == "none": - c.logger.SetOutput(ioutil.Discard) + c.globalState.logger.SetOutput(ioutil.Discard) case strings.HasPrefix(line, "loki"): ch = make(chan struct{}) // TODO: refactor, get it from the constructor - hook, err := log.LokiFromConfigLine(c.ctx, c.fallbackLogger, line, ch) + hook, err := log.LokiFromConfigLine(c.globalState.ctx, c.globalState.fallbackLogger, line, ch) if err != nil { return nil, err } - c.logger.AddHook(hook) - c.logger.SetOutput(ioutil.Discard) // don't output to anywhere else - c.logFmt = "raw" + c.globalState.logger.AddHook(hook) + c.globalState.logger.SetOutput(ioutil.Discard) // don't output to anywhere else + c.globalState.flags.logFormat = "raw" case strings.HasPrefix(line, "file"): ch = make(chan struct{}) // TODO: refactor, get it from the constructor - hook, err := log.FileHookFromConfigLine(c.ctx, c.fallbackLogger, line, ch) + hook, err := log.FileHookFromConfigLine(c.globalState.ctx, c.globalState.fs, c.globalState.fallbackLogger, line, ch) if err != nil { return nil, err } - c.logger.AddHook(hook) - c.logger.SetOutput(ioutil.Discard) + c.globalState.logger.AddHook(hook) + c.globalState.logger.SetOutput(ioutil.Discard) default: - return nil, fmt.Errorf("unsupported log output `%s`", line) + return nil, fmt.Errorf("unsupported log output '%s'", line) } - switch c.logFmt { + switch c.globalState.flags.logFormat { case "raw": - c.logger.SetFormatter(&RawFormatter{}) - c.logger.Debug("Logger format: RAW") + c.globalState.logger.SetFormatter(&RawFormatter{}) + c.globalState.logger.Debug("Logger format: RAW") case "json": - c.logger.SetFormatter(&logrus.JSONFormatter{}) - c.logger.Debug("Logger format: JSON") + c.globalState.logger.SetFormatter(&logrus.JSONFormatter{}) + c.globalState.logger.Debug("Logger format: JSON") default: - c.logger.SetFormatter(&logrus.TextFormatter{ForceColors: loggerForceColors, DisableColors: c.commandFlags.noColor}) - c.logger.Debug("Logger format: TEXT") + c.globalState.logger.SetFormatter(&logrus.TextFormatter{ + ForceColors: loggerForceColors, DisableColors: c.globalState.flags.noColor, + }) + c.globalState.logger.Debug("Logger format: TEXT") } return ch, nil } diff --git a/cmd/root_test.go b/cmd/root_test.go new file mode 100644 index 00000000000..f8ecaeb6f6c --- /dev/null +++ b/cmd/root_test.go @@ -0,0 +1,73 @@ +package cmd + +import ( + "bytes" + "context" + "os" + "os/signal" + "runtime" + "sync" + "testing" + + "github.com/sirupsen/logrus" + "github.com/spf13/afero" + "github.com/stretchr/testify/require" + "go.k6.io/k6/lib/testutils" +) + +type globalTestState struct { + *globalState + cancel func() + + stdOut, stdErr *bytes.Buffer + loggerHook *testutils.SimpleLogrusHook + + cwd string +} + +func newGlobalTestState(t *testing.T) *globalTestState { + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + fs := &afero.MemMapFs{} + cwd := "/test/" + if runtime.GOOS == "windows" { + cwd = "c:\\test\\" + } + require.NoError(t, fs.MkdirAll(cwd, 0o755)) + + logger := logrus.New() + logger.SetLevel(logrus.InfoLevel) + logger.Out = testutils.NewTestOutput(t) + hook := &testutils.SimpleLogrusHook{HookedLevels: logrus.AllLevels} + logger.AddHook(hook) + + ts := &globalTestState{ + cwd: cwd, + cancel: cancel, + loggerHook: hook, + stdOut: new(bytes.Buffer), + stdErr: new(bytes.Buffer), + } + + outMutex := &sync.Mutex{} + defaultFlags := getDefaultFlags(".config") + ts.globalState = &globalState{ + ctx: ctx, + fs: fs, + getwd: func() (string, error) { return ts.cwd, nil }, + args: []string{}, + envVars: map[string]string{}, + defaultFlags: defaultFlags, + flags: defaultFlags, + outMutex: outMutex, + stdOut: &consoleWriter{nil, ts.stdOut, false, outMutex, nil}, + stdErr: &consoleWriter{nil, ts.stdErr, false, outMutex, nil}, + stdIn: os.Stdin, // TODO: spoof? + signalNotify: signal.Notify, + signalStop: signal.Stop, + logger: logger, + fallbackLogger: testutils.NewLogger(t).WithField("fallback", true), + } + return ts +} diff --git a/cmd/run.go b/cmd/run.go index 6e168823991..2c73130bd31 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -29,7 +29,6 @@ import ( "io" "net/http" "os" - "os/signal" "runtime" "sync" "syscall" @@ -60,7 +59,7 @@ const ( ) //nolint:funlen,gocognit,gocyclo,cyclop -func getRunCmd(ctx context.Context, logger *logrus.Logger, globalFlags *commandFlags) *cobra.Command { +func getRunCmd(globalState *globalState) *cobra.Command { // runCmd represents the run command. runCmd := &cobra.Command{ Use: "run", @@ -90,25 +89,27 @@ a commandline interface for interacting with it.`, 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 { // TODO: disable in quiet mode? - _, _ = fmt.Fprintf(globalFlags.stdout, "\n%s\n\n", getBanner(globalFlags.noColor || !globalFlags.stdoutTTY)) + _, _ = fmt.Fprintf(globalState.stdOut, "\n%s\n\n", getBanner(globalState.flags.noColor || !globalState.stdOut.isTTY)) + logger := globalState.logger logger.Debug("Initializing the runner...") // Create the Runner. - src, filesystems, err := readSource(args[0], logger) + src, filesystems, err := readSource(globalState, args[0]) if err != nil { return err } - osEnvironment := buildEnvMap(os.Environ()) - runtimeOptions, err := getRuntimeOptions(cmd.Flags(), osEnvironment) + runtimeOptions, err := getRuntimeOptions(cmd.Flags(), globalState.envVars) if err != nil { return err } registry := metrics.NewRegistry() builtinMetrics := metrics.RegisterBuiltinMetrics(registry) - initRunner, err := newRunner(logger, src, globalFlags.runType, filesystems, runtimeOptions, builtinMetrics, registry) + initRunner, err := newRunner( + logger, src, globalState.flags.runType, filesystems, runtimeOptions, builtinMetrics, registry, + ) if err != nil { return common.UnwrapGojaInterruptedError(err) } @@ -119,8 +120,7 @@ a commandline interface for interacting with it.`, if err != nil { return err } - conf, err := getConsolidatedConfig( - afero.NewOsFs(), cliConf, initRunner.GetOptions(), buildEnvMap(os.Environ()), globalFlags) + conf, err := getConsolidatedConfig(globalState, cliConf, initRunner.GetOptions()) if err != nil { return err } @@ -157,7 +157,7 @@ a commandline interface for interacting with it.`, // - The globalCtx is cancelled only after we're completely done with the // test execution and any --linger has been cleared, so that the Engine // can start winding down its metrics processing. - globalCtx, globalCancel := context.WithCancel(ctx) + globalCtx, globalCancel := context.WithCancel(globalState.ctx) defer globalCancel() lingerCtx, lingerCancel := context.WithCancel(globalCtx) defer lingerCancel() @@ -185,13 +185,13 @@ a commandline interface for interacting with it.`, for _, s := range execScheduler.GetExecutors() { pbs = append(pbs, s.GetProgress()) } - showProgress(progressCtx, pbs, logger, globalFlags) + showProgress(progressCtx, globalState, pbs, logger) progressBarWG.Done() }() // Create all outputs. executionPlan := execScheduler.GetExecutionPlan() - outputs, err := createOutputs(conf.Out, src, conf, runtimeOptions, executionPlan, osEnvironment, logger, globalFlags) + outputs, err := createOutputs(globalState, src, conf, runtimeOptions, executionPlan) if err != nil { return err } @@ -204,11 +204,11 @@ a commandline interface for interacting with it.`, } // Spin up the REST API server, if not disabled. - if globalFlags.address != "" { + if globalState.flags.address != "" { initBar.Modify(pb.WithConstProgress(0, "Init API server")) go func() { - logger.Debugf("Starting the REST API server on %s", globalFlags.address) - if aerr := api.ListenAndServe(globalFlags.address, engine, logger); aerr != nil { + logger.Debugf("Starting the REST API server on %s", globalState.flags.address) + if aerr := api.ListenAndServe(globalState.flags.address, engine, logger); aerr != nil { // Only exit k6 if the user has explicitly set the REST API address if cmd.Flags().Lookup("address").Changed { logger.WithError(aerr).Error("Error from API server") @@ -229,13 +229,13 @@ a commandline interface for interacting with it.`, defer engine.StopOutputs() printExecutionDescription( - "local", args[0], "", conf, execScheduler.GetState().ExecutionTuple, - executionPlan, outputs, globalFlags.noColor || !globalFlags.stdoutTTY, globalFlags) + globalState, "local", args[0], "", conf, execScheduler.GetState().ExecutionTuple, executionPlan, outputs, + ) // Trap Interrupts, SIGINTs and SIGTERMs. - sigC := make(chan os.Signal, 1) - signal.Notify(sigC, os.Interrupt, syscall.SIGINT, syscall.SIGTERM) - defer signal.Stop(sigC) + sigC := make(chan os.Signal, 2) + globalState.signalNotify(sigC, os.Interrupt, syscall.SIGINT, syscall.SIGTERM) + defer globalState.signalStop(sigC) go func() { sig := <-sigC logger.WithField("sig", sig).Debug("Stopping k6 in response to signal...") @@ -308,14 +308,14 @@ a commandline interface for interacting with it.`, Metrics: engine.Metrics, RootGroup: engine.ExecutionScheduler.GetRunner().GetDefaultGroup(), TestRunDuration: executionState.GetCurrentTestRunDuration(), - NoColor: globalFlags.noColor, + NoColor: globalState.flags.noColor, UIState: lib.UIState{ - IsStdOutTTY: globalFlags.stdoutTTY, - IsStdErrTTY: globalFlags.stderrTTY, + IsStdOutTTY: globalState.stdOut.isTTY, + IsStdErrTTY: globalState.stdErr.isTTY, }, }) if err == nil { - err = handleSummaryResult(afero.NewOsFs(), globalFlags.stdout, globalFlags.stderr, summaryResult) + err = handleSummaryResult(globalState.fs, globalState.stdOut, globalState.stdErr, summaryResult) } if err != nil { logger.WithError(err).Error("failed to handle the end-of-test summary") @@ -328,7 +328,7 @@ a commandline interface for interacting with it.`, // do nothing, we were interrupted by Ctrl+C already default: logger.Debug("Linger set; waiting for Ctrl+C...") - fprintf(globalFlags.stdout, "Linger set; waiting for Ctrl+C...") + fprintf(globalState.stdOut, "Linger set; waiting for Ctrl+C...") <-lingerCtx.Done() logger.Debug("Ctrl+C received, exiting...") } @@ -348,7 +348,7 @@ a commandline interface for interacting with it.`, } runCmd.Flags().SortFlags = false - runCmd.Flags().AddFlagSet(runCmdFlagSet(globalFlags)) + runCmd.Flags().AddFlagSet(runCmdFlagSet(globalState)) return runCmd } @@ -384,7 +384,7 @@ func reportUsage(execScheduler *local.ExecutionScheduler) error { return err } -func runCmdFlagSet(globalFlags *commandFlags) *pflag.FlagSet { +func runCmdFlagSet(globalState *globalState) *pflag.FlagSet { flags := pflag.NewFlagSet("", pflag.ContinueOnError) flags.SortFlags = false flags.AddFlagSet(optionFlagSet()) @@ -398,7 +398,8 @@ func runCmdFlagSet(globalFlags *commandFlags) *pflag.FlagSet { // that will be used in the help/usage message - if we don't set it, the environment // variables will affect the usage message // - and finally, global variables are not very testable... :/ - flags.StringVarP(&globalFlags.runType, "type", "t", globalFlags.runType, "override file `type`, \"js\" or \"archive\"") + flags.StringVarP(&globalState.flags.runType, "type", "t", + globalState.flags.runType, "override file `type`, \"js\" or \"archive\"") flags.Lookup("type").DefValue = "" return flags } diff --git a/cmd/run_test.go b/cmd/run_test.go index 2c33d0e365d..a11e9800de2 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -22,8 +22,8 @@ package cmd import ( "bytes" - "context" "errors" + "fmt" "io" "io/ioutil" "os" @@ -33,7 +33,6 @@ import ( "strings" "testing" - "github.com/sirupsen/logrus" "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -42,7 +41,6 @@ import ( "go.k6.io/k6/errext/exitcodes" "go.k6.io/k6/js/common" "go.k6.io/k6/lib/fsext" - "go.k6.io/k6/lib/testutils" ) type mockWriter struct { @@ -134,55 +132,88 @@ func TestHandleSummaryResultError(t *testing.T) { assertEqual(t, "file summary 2", files[filePath2]) } -func TestAbortTest(t *testing.T) { +func TestRunScriptErrorsAndAbort(t *testing.T) { t.Parallel() testCases := []struct { - testFilename, expLogOutput string + testFilename, name string + expErr, expLogOutput string + expExitCode errext.ExitCode + extraArgs []string }{ { testFilename: "abort.js", + expErr: common.AbortTest, + expExitCode: exitcodes.ScriptAborted, }, { testFilename: "abort_initerr.js", + expErr: common.AbortTest, + expExitCode: exitcodes.ScriptAborted, }, { testFilename: "abort_initvu.js", + expErr: common.AbortTest, + expExitCode: exitcodes.ScriptAborted, }, { testFilename: "abort_teardown.js", + expErr: common.AbortTest, + expExitCode: exitcodes.ScriptAborted, expLogOutput: "Calling teardown function after test.abort()", }, + { + testFilename: "initerr.js", + expErr: "ReferenceError: someUndefinedVar is not defined", + expExitCode: exitcodes.ScriptException, + }, + { + testFilename: "thresholds/malformed_expression.js", + name: "run should fail with exit status 104 on a malformed threshold expression", + expErr: "malformed threshold expression", + expExitCode: exitcodes.InvalidConfig, + }, + { + testFilename: "thresholds/malformed_expression.js", + name: "run should on a malformed threshold expression but --no-thresholds flag set", + extraArgs: []string{"--no-thresholds"}, + // we don't expect an error + }, } for _, tc := range testCases { tc := tc - t.Run(tc.testFilename, func(t *testing.T) { + name := tc.testFilename + if tc.name != "" { + name = fmt.Sprintf("%s (%s)", tc.testFilename, tc.name) + } + t.Run(name, func(t *testing.T) { t.Parallel() - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - logger := logrus.New() - logger.SetLevel(logrus.InfoLevel) - logger.Out = ioutil.Discard - hook := testutils.SimpleLogrusHook{ - HookedLevels: []logrus.Level{logrus.InfoLevel}, + testScript, err := ioutil.ReadFile(path.Join("testdata", tc.testFilename)) + require.NoError(t, err) + + testState := newGlobalTestState(t) + require.NoError(t, afero.WriteFile(testState.fs, filepath.Join(testState.cwd, tc.testFilename), testScript, 0o644)) + testState.args = append([]string{"k6", "run", tc.testFilename}, tc.extraArgs...) + + err = newRootCommand(testState.globalState).cmd.Execute() + + if tc.expErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expErr) + } else { + require.NoError(t, err) } - logger.AddHook(&hook) - cmd := getRunCmd(ctx, logger, newCommandFlags()) - a, err := filepath.Abs(path.Join("testdata", tc.testFilename)) - require.NoError(t, err) - cmd.SetArgs([]string{a}) - err = cmd.Execute() - var e errext.HasExitCode - require.ErrorAs(t, err, &e) - assert.Equalf(t, exitcodes.ScriptAborted, e.ExitCode(), - "Status code must be %d", exitcodes.ScriptAborted) - assert.Contains(t, e.Error(), common.AbortTest) + if tc.expExitCode != 0 { + var e errext.HasExitCode + require.ErrorAs(t, err, &e) + assert.Equalf(t, tc.expExitCode, e.ExitCode(), "Status code must be %d", tc.expExitCode) + } if tc.expLogOutput != "" { var gotMsg bool - for _, entry := range hook.Drain() { + for _, entry := range testState.loggerHook.Drain() { if strings.Contains(entry.Message, tc.expLogOutput) { gotMsg = true break @@ -193,81 +224,3 @@ func TestAbortTest(t *testing.T) { }) } } - -func TestInitErrExitCode(t *testing.T) { - t.Parallel() - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - logger := testutils.NewLogger(t) - - cmd := getRunCmd(ctx, logger, newCommandFlags()) - a, err := filepath.Abs("testdata/initerr.js") - require.NoError(t, err) - cmd.SetArgs([]string{a}) - err = cmd.Execute() - var e errext.HasExitCode - require.ErrorAs(t, err, &e) - assert.Equalf(t, exitcodes.ScriptException, e.ExitCode(), - "Status code must be %d", exitcodes.ScriptException) - assert.Contains(t, err.Error(), "ReferenceError: someUndefinedVar is not defined") -} - -func TestRunThresholds(t *testing.T) { - t.Parallel() - - testCases := []struct { - name string - noThresholds bool - testFilename string - - wantErr bool - }{ - { - name: "run should fail with exit status 104 on a malformed threshold expression", - noThresholds: false, - testFilename: "testdata/thresholds/malformed_expression.js", - wantErr: true, - }, - { - name: "run should on a malformed threshold expression but --no-thresholds flag set", - noThresholds: true, - testFilename: "testdata/thresholds/malformed_expression.js", - wantErr: false, - }, - } - - for _, testCase := range testCases { - testCase := testCase - t.Run(testCase.name, func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - cmd := getRunCmd(ctx, testutils.NewLogger(t), newCommandFlags()) - filename, err := filepath.Abs(testCase.testFilename) - require.NoError(t, err) - args := []string{filename} - if testCase.noThresholds { - args = append(args, "--no-thresholds") - } - cmd.SetArgs(args) - wantExitCode := exitcodes.InvalidConfig - - var gotErrExt errext.HasExitCode - gotErr := cmd.Execute() - - assert.Equal(t, - testCase.wantErr, - gotErr != nil, - "run command error = %v, wantErr %v", gotErr, testCase.wantErr, - ) - - if testCase.wantErr { - require.ErrorAs(t, gotErr, &gotErrExt) - assert.Equalf(t, wantExitCode, gotErrExt.ExitCode(), - "status code must be %d", wantExitCode, - ) - } - }) - } -} diff --git a/cmd/runtime_options.go b/cmd/runtime_options.go index 4b5b6ff7ede..47214875cf3 100644 --- a/cmd/runtime_options.go +++ b/cmd/runtime_options.go @@ -24,7 +24,6 @@ import ( "fmt" "regexp" "strconv" - "strings" "github.com/spf13/pflag" "gopkg.in/guregu/null.v3" @@ -38,22 +37,6 @@ import ( var userEnvVarName = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]*$`) -func parseEnvKeyValue(kv string) (string, string) { - if idx := strings.IndexRune(kv, '='); idx != -1 { - return kv[:idx], kv[idx+1:] - } - return kv, "" -} - -func buildEnvMap(environ []string) map[string]string { - env := make(map[string]string, len(environ)) - for _, kv := range environ { - k, v := parseEnvKeyValue(kv) - env[k] = v - } - return env -} - func runtimeOptionFlagSet(includeSysEnv bool) *pflag.FlagSet { flags := pflag.NewFlagSet("", 0) flags.SortFlags = false diff --git a/cmd/scale.go b/cmd/scale.go index 1ba14615b77..d2e29da838f 100644 --- a/cmd/scale.go +++ b/cmd/scale.go @@ -21,7 +21,6 @@ package cmd import ( - "context" "errors" "github.com/spf13/cobra" @@ -30,7 +29,7 @@ import ( "go.k6.io/k6/api/v1/client" ) -func getScaleCmd(ctx context.Context, globalFlags *commandFlags) *cobra.Command { +func getScaleCmd(globalState *globalState) *cobra.Command { // scaleCmd represents the scale command scaleCmd := &cobra.Command{ Use: "scale", @@ -45,16 +44,16 @@ func getScaleCmd(ctx context.Context, globalFlags *commandFlags) *cobra.Command return errors.New("Specify either -u/--vus or -m/--max") //nolint:golint,stylecheck } - c, err := client.New(globalFlags.address) + c, err := client.New(globalState.flags.address) if err != nil { return err } - status, err := c.SetStatus(ctx, v1.Status{VUs: vus, VUsMax: max}) + status, err := c.SetStatus(globalState.ctx, v1.Status{VUs: vus, VUsMax: max}) if err != nil { return err } - return yamlPrint(globalFlags.stdout, status) + return yamlPrint(globalState.stdOut, status) }, } diff --git a/cmd/stats.go b/cmd/stats.go index 1d521131808..e375926b7f1 100644 --- a/cmd/stats.go +++ b/cmd/stats.go @@ -21,14 +21,12 @@ package cmd import ( - "context" - "github.com/spf13/cobra" "go.k6.io/k6/api/v1/client" ) -func getStatsCmd(ctx context.Context, globalFlags *commandFlags) *cobra.Command { +func getStatsCmd(globalState *globalState) *cobra.Command { // statsCmd represents the stats command statsCmd := &cobra.Command{ Use: "stats", @@ -37,16 +35,16 @@ func getStatsCmd(ctx context.Context, globalFlags *commandFlags) *cobra.Command Use the global --address flag to specify the URL to the API server.`, RunE: func(cmd *cobra.Command, args []string) error { - c, err := client.New(globalFlags.address) + c, err := client.New(globalState.flags.address) if err != nil { return err } - metrics, err := c.Metrics(ctx) + metrics, err := c.Metrics(globalState.ctx) if err != nil { return err } - return yamlPrint(globalFlags.stdout, metrics) + return yamlPrint(globalState.stdOut, metrics) }, } return statsCmd diff --git a/cmd/status.go b/cmd/status.go index 87ee697da82..3f90fc67ef1 100644 --- a/cmd/status.go +++ b/cmd/status.go @@ -21,14 +21,12 @@ package cmd import ( - "context" - "github.com/spf13/cobra" "go.k6.io/k6/api/v1/client" ) -func getStatusCmd(ctx context.Context, globalFlags *commandFlags) *cobra.Command { +func getStatusCmd(globalState *globalState) *cobra.Command { // statusCmd represents the status command statusCmd := &cobra.Command{ Use: "status", @@ -37,16 +35,16 @@ func getStatusCmd(ctx context.Context, globalFlags *commandFlags) *cobra.Command Use the global --address flag to specify the URL to the API server.`, RunE: func(cmd *cobra.Command, args []string) error { - c, err := client.New(globalFlags.address) + c, err := client.New(globalState.flags.address) if err != nil { return err } - status, err := c.Status(ctx) + status, err := c.Status(globalState.ctx) if err != nil { return err } - return yamlPrint(globalFlags.stdout, status) + return yamlPrint(globalState.stdOut, status) }, } return statusCmd diff --git a/cmd/ui.go b/cmd/ui.go index 87f3e80fdf0..1f63bb22189 100644 --- a/cmd/ui.go +++ b/cmd/ui.go @@ -26,7 +26,6 @@ import ( "fmt" "io" "os" - "os/signal" "strings" "sync" "time" @@ -55,28 +54,29 @@ const ( // A writer that syncs writes with a mutex and, if the output is a TTY, clears before newlines. type consoleWriter struct { - Writer io.Writer - IsTTY bool - Mutex *sync.Mutex + rawOut *os.File + writer io.Writer + isTTY bool + mutex *sync.Mutex // Used for flicker-free persistent objects like the progressbars - PersistentText func() + persistentText func() } func (w *consoleWriter) Write(p []byte) (n int, err error) { origLen := len(p) - if w.IsTTY { + if w.isTTY { // Add a TTY code to erase till the end of line with each new line // TODO: check how cross-platform this is... p = bytes.ReplaceAll(p, []byte{'\n'}, []byte{'\x1b', '[', '0', 'K', '\n'}) } - w.Mutex.Lock() - n, err = w.Writer.Write(p) - if w.PersistentText != nil { - w.PersistentText() + w.mutex.Lock() + n, err = w.writer.Write(p) + if w.persistentText != nil { + w.persistentText() } - w.Mutex.Unlock() + w.mutex.Unlock() if err != nil && n < origLen { return n, err @@ -104,8 +104,8 @@ func getBanner(noColor bool) string { return c.Sprint(consts.Banner()) } -func printBar(bar *pb.ProgressBar, globalFlags *commandFlags) { - if globalFlags.quiet { +func printBar(gs *globalState, bar *pb.ProgressBar) { + if gs.flags.quiet { return } end := "\n" @@ -113,7 +113,7 @@ func printBar(bar *pb.ProgressBar, globalFlags *commandFlags) { // stateless... basically first render the left and right parts, so we know // how long the longest line is, and how much space we have for the progress widthDelta := -defaultTermWidth - if globalFlags.stdout.IsTTY { + if gs.stdOut.isTTY { // If we're in a TTY, instead of printing the bar and going to the next // line, erase everything till the end of the line and return to the // start, so that the next print will overwrite the same line. @@ -124,24 +124,25 @@ func printBar(bar *pb.ProgressBar, globalFlags *commandFlags) { } rendered := bar.Render(0, widthDelta) // Only output the left and middle part of the progress bar - fprintf(globalFlags.stdout, "%s%s", rendered.String(), end) + fprintf(gs.stdOut, "%s%s", rendered.String(), end) } -func modifyAndPrintBar(bar *pb.ProgressBar, globalFlags *commandFlags, options ...pb.ProgressBarOption) { +func modifyAndPrintBar(gs *globalState, bar *pb.ProgressBar, options ...pb.ProgressBarOption) { bar.Modify(options...) - printBar(bar, globalFlags) + printBar(gs, bar) } // Print execution description for both cloud and local execution. // TODO: Clean this up as part of #1499 or #1427 func printExecutionDescription( - execution, filename, outputOverride string, conf Config, et *lib.ExecutionTuple, - execPlan []lib.ExecutionStep, outputs []output.Output, noColor bool, globalFlags *commandFlags, + gs *globalState, execution, filename, outputOverride string, conf Config, + et *lib.ExecutionTuple, execPlan []lib.ExecutionStep, outputs []output.Output, ) { + noColor := gs.flags.noColor || !gs.stdOut.isTTY valueColor := getColor(noColor, color.FgCyan) - fprintf(globalFlags.stdout, " execution: %s\n", valueColor.Sprint(execution)) - fprintf(globalFlags.stdout, " script: %s\n", valueColor.Sprint(filename)) + fprintf(gs.stdOut, " execution: %s\n", valueColor.Sprint(execution)) + fprintf(gs.stdOut, " script: %s\n", valueColor.Sprint(filename)) var outputDescriptions []string switch { @@ -155,8 +156,8 @@ func printExecutionDescription( } } - fprintf(globalFlags.stdout, " output: %s\n", valueColor.Sprint(strings.Join(outputDescriptions, ", "))) - fprintf(globalFlags.stdout, "\n") + fprintf(gs.stdOut, " output: %s\n", valueColor.Sprint(strings.Join(outputDescriptions, ", "))) + fprintf(gs.stdOut, "\n") maxDuration, _ := lib.GetEndOffset(execPlan) executorConfigs := conf.Scenarios.GetSortedConfigs() @@ -166,21 +167,21 @@ func printExecutionDescription( scenarioDesc = fmt.Sprintf("%d scenarios", len(executorConfigs)) } - fprintf(globalFlags.stdout, " scenarios: %s\n", valueColor.Sprintf( + fprintf(gs.stdOut, " scenarios: %s\n", valueColor.Sprintf( "(%.2f%%) %s, %d max VUs, %s max duration (incl. graceful stop):", conf.ExecutionSegment.FloatLength()*100, scenarioDesc, lib.GetMaxPossibleVUs(execPlan), maxDuration.Round(100*time.Millisecond)), ) for _, ec := range executorConfigs { - fprintf(globalFlags.stdout, " * %s: %s\n", + fprintf(gs.stdOut, " * %s: %s\n", ec.GetName(), ec.GetDescription(et)) } - fprintf(globalFlags.stdout, "\n") + fprintf(gs.stdOut, "\n") } //nolint: funlen func renderMultipleBars( - isTTY, goBack bool, maxLeft, termWidth, widthDelta int, pbs []*pb.ProgressBar, globalFlags *commandFlags, + nocolor, isTTY, goBack bool, maxLeft, termWidth, widthDelta int, pbs []*pb.ProgressBar, ) (string, int) { lineEnd := "\n" if isTTY { @@ -248,7 +249,7 @@ func renderMultipleBars( longestLine = lineRuneCount } lineBreaks += (lineRuneCount - termPadding) / termWidth - if !globalFlags.noColor { + if !nocolor { rend.Color = true status = fmt.Sprintf(" %s ", rend.Status()) line = fmt.Sprintf(leftPadFmt+"%s%s%s", @@ -272,15 +273,15 @@ func renderMultipleBars( // TODO: add a no-progress option that will disable these // TODO: don't use global variables... // nolint:funlen,gocognit -func showProgress(ctx context.Context, pbs []*pb.ProgressBar, logger *logrus.Logger, globalFlags *commandFlags) { - if globalFlags.quiet { +func showProgress(ctx context.Context, gs *globalState, pbs []*pb.ProgressBar, logger *logrus.Logger) { + if gs.flags.quiet { return } var errTermGetSize bool termWidth := defaultTermWidth - if globalFlags.stdoutTTY { - tw, _, err := term.GetSize(int(os.Stdout.Fd())) + if gs.stdOut.isTTY { + tw, _, err := term.GetSize(int(gs.stdOut.rawOut.Fd())) if !(tw > 0) || err != nil { errTermGetSize = true logger.WithError(err).Warn("error getting terminal size") @@ -304,7 +305,7 @@ func showProgress(ctx context.Context, pbs []*pb.ProgressBar, logger *logrus.Log printProgressBars := func() { progressBarsLastRenderLock.Lock() - _, _ = globalFlags.stdout.Writer.Write(progressBarsLastRender) + _, _ = gs.stdOut.writer.Write(progressBarsLastRender) progressBarsLastRenderLock.Unlock() } @@ -312,7 +313,8 @@ func showProgress(ctx context.Context, pbs []*pb.ProgressBar, logger *logrus.Log // Default to responsive progress bars when in an interactive terminal renderProgressBars := func(goBack bool) { barText, longestLine := renderMultipleBars( - globalFlags.stdoutTTY, goBack, maxLeft, termWidth, widthDelta, pbs, globalFlags) + gs.flags.noColor, gs.stdOut.isTTY, goBack, maxLeft, termWidth, widthDelta, pbs, + ) widthDelta = termWidth - longestLine - termPadding progressBarsLastRenderLock.Lock() progressBarsLastRender = []byte(barText) @@ -320,10 +322,10 @@ func showProgress(ctx context.Context, pbs []*pb.ProgressBar, logger *logrus.Log } // Otherwise fallback to fixed compact progress bars - if !globalFlags.stdoutTTY { + if !gs.stdOut.isTTY { widthDelta = -pb.DefaultWidth renderProgressBars = func(goBack bool) { - barText, _ := renderMultipleBars(globalFlags.stdoutTTY, goBack, maxLeft, termWidth, widthDelta, pbs, globalFlags) + barText, _ := renderMultipleBars(gs.flags.noColor, gs.stdOut.isTTY, goBack, maxLeft, termWidth, widthDelta, pbs) progressBarsLastRenderLock.Lock() progressBarsLastRender = []byte(barText) progressBarsLastRenderLock.Unlock() @@ -332,61 +334,60 @@ func showProgress(ctx context.Context, pbs []*pb.ProgressBar, logger *logrus.Log // TODO: make configurable? updateFreq := 1 * time.Second - if globalFlags.stdoutTTY { + var stdoutFD int + if gs.stdOut.isTTY { + stdoutFD = int(gs.stdOut.rawOut.Fd()) updateFreq = 100 * time.Millisecond - globalFlags.outMutex.Lock() - globalFlags.stdout.PersistentText = printProgressBars - globalFlags.stderr.PersistentText = printProgressBars - globalFlags.outMutex.Unlock() + gs.outMutex.Lock() + gs.stdOut.persistentText = printProgressBars + gs.stdErr.persistentText = printProgressBars + gs.outMutex.Unlock() defer func() { - globalFlags.outMutex.Lock() - globalFlags.stdout.PersistentText = nil - globalFlags.stderr.PersistentText = nil - globalFlags.outMutex.Unlock() + gs.outMutex.Lock() + gs.stdOut.persistentText = nil + gs.stdErr.persistentText = nil + gs.outMutex.Unlock() }() } - var ( - fd = int(os.Stdout.Fd()) - ticker = time.NewTicker(updateFreq) - ) - var winch chan os.Signal if sig := getWinchSignal(); sig != nil { - winch = make(chan os.Signal, 1) - signal.Notify(winch, sig) + winch = make(chan os.Signal, 10) + gs.signalNotify(winch, sig) + defer gs.signalStop(winch) } + ticker := time.NewTicker(updateFreq) ctxDone := ctx.Done() for { select { case <-ctxDone: renderProgressBars(false) - globalFlags.outMutex.Lock() + gs.outMutex.Lock() printProgressBars() - globalFlags.outMutex.Unlock() + gs.outMutex.Unlock() return case <-winch: - if globalFlags.stdoutTTY && !errTermGetSize { + if gs.stdOut.isTTY && !errTermGetSize { // More responsive progress bar resizing on platforms with SIGWINCH (*nix) - tw, _, err := term.GetSize(fd) + tw, _, err := term.GetSize(stdoutFD) if tw > 0 && err == nil { termWidth = tw } } case <-ticker.C: // Default ticker-based progress bar resizing - if globalFlags.stdoutTTY && !errTermGetSize && winch == nil { - tw, _, err := term.GetSize(fd) + if gs.stdOut.isTTY && !errTermGetSize && winch == nil { + tw, _, err := term.GetSize(stdoutFD) if tw > 0 && err == nil { termWidth = tw } } } renderProgressBars(true) - globalFlags.outMutex.Lock() + gs.outMutex.Lock() printProgressBars() - globalFlags.outMutex.Unlock() + gs.outMutex.Unlock() } } diff --git a/cmd/ui_test.go b/cmd/ui_test.go index 55f8eec61f7..c9e9e79780b 100644 --- a/cmd/ui_test.go +++ b/cmd/ui_test.go @@ -89,7 +89,7 @@ left 2 [ 0% ] right 2 000 t.Run(tc.name, func(t *testing.T) { t.Parallel() pbs := createTestProgressBars(3, tc.padding, 1) - out, longestLine := renderMultipleBars(false, false, 6+tc.padding, 80, tc.widthDelta, pbs, &commandFlags{}) + out, longestLine := renderMultipleBars(true, false, false, 6+tc.padding, 80, tc.widthDelta, pbs) assert.Equal(t, tc.expOut, out) assert.Equal(t, tc.expLongLine, longestLine) }) diff --git a/cmd/version.go b/cmd/version.go index 52efb696290..c9b4956cb16 100644 --- a/cmd/version.go +++ b/cmd/version.go @@ -21,21 +21,19 @@ package cmd import ( - "fmt" - "github.com/spf13/cobra" "go.k6.io/k6/lib/consts" ) -func getVersionCmd() *cobra.Command { +func getVersionCmd(globalState *globalState) *cobra.Command { // versionCmd represents the version command. versionCmd := &cobra.Command{ Use: "version", Short: "Show application version", Long: `Show the application version and exit.`, Run: func(_ *cobra.Command, _ []string) { - fmt.Println("k6 v" + consts.FullVersion()) //nolint:forbidigo // we probably shouldn't do that though + fprintf(globalState.stdOut, "k6 v%s\n", consts.FullVersion()) }, } return versionCmd diff --git a/loader/filesystems.go b/loader/filesystems.go index ddf8dd91a5b..aadbe94d066 100644 --- a/loader/filesystems.go +++ b/loader/filesystems.go @@ -29,12 +29,11 @@ import ( ) // CreateFilesystems creates the correct filesystem map for the current OS -func CreateFilesystems() map[string]afero.Fs { +func CreateFilesystems(osfs afero.Fs) map[string]afero.Fs { // We want to eliminate disk access at runtime, so we set up a memory mapped cache that's // written every time something is read from the real filesystem. This cache is then used for // successive spawns to read from (they have no access to the real disk). // Also initialize the same for `https` but the caching is handled manually in the loader package - osfs := afero.NewOsFs() if runtime.GOOS == "windows" { // This is done so that we can continue to use paths with /|"\" through the code but also to // be easier to traverse the cachedFs later as it doesn't work very well if you have windows diff --git a/log/file.go b/log/file.go index 91fbc31c7dd..2901c3f367c 100644 --- a/log/file.go +++ b/log/file.go @@ -31,6 +31,7 @@ import ( "strings" "github.com/sirupsen/logrus" + "github.com/spf13/afero" ) // fileHookBufferSize is a default size for the fileHook's loglines channel. @@ -38,6 +39,7 @@ const fileHookBufferSize = 100 // fileHook is a hook to handle writing to local files. type fileHook struct { + fs afero.Fs fallbackLogger logrus.FieldLogger loglines chan []byte path string @@ -49,11 +51,12 @@ type fileHook struct { // FileHookFromConfigLine returns new fileHook hook. func FileHookFromConfigLine( - ctx context.Context, fallbackLogger logrus.FieldLogger, line string, done chan struct{}, + ctx context.Context, fs afero.Fs, fallbackLogger logrus.FieldLogger, line string, done chan struct{}, ) (logrus.Hook, error) { // TODO: fix this so it works correctly with relative paths from the CWD hook := &fileHook{ + fs: fs, fallbackLogger: fallbackLogger, levels: logrus.AllLevels, done: done, @@ -105,11 +108,11 @@ func (h *fileHook) parseArgs(line string) error { // openFile opens logfile and initializes writers. func (h *fileHook) openFile() error { - if _, err := os.Stat(filepath.Dir(h.path)); os.IsNotExist(err) { + if _, err := h.fs.Stat(filepath.Dir(h.path)); os.IsNotExist(err) { return fmt.Errorf("provided directory '%s' does not exist", filepath.Dir(h.path)) } - file, err := os.OpenFile(h.path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o600) + file, err := h.fs.OpenFile(h.path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o600) if err != nil { return fmt.Errorf("failed to open logfile %s: %w", h.path, err) } diff --git a/log/file_test.go b/log/file_test.go index 49a36ed893d..ca4417f2bc5 100644 --- a/log/file_test.go +++ b/log/file_test.go @@ -24,13 +24,12 @@ import ( "bufio" "bytes" "context" - "fmt" "io" - "os" "testing" "time" "github.com/sirupsen/logrus" + "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -62,17 +61,13 @@ func TestFileHookFromConfigLine(t *testing.T) { }, }, { - line: fmt.Sprintf("file=%s/k6.log,level=info", os.TempDir()), + line: "file=/k6.log,level=info", err: false, res: fileHook{ - path: fmt.Sprintf("%s/k6.log", os.TempDir()), + path: "/k6.log", levels: logrus.AllLevels[:5], }, }, - { - line: "file=./", - err: true, - }, { line: "file=/a/c/", err: true, @@ -116,7 +111,7 @@ func TestFileHookFromConfigLine(t *testing.T) { t.Parallel() res, err := FileHookFromConfigLine( - context.Background(), logrus.New(), test.line, make(chan struct{}), + context.Background(), afero.NewMemMapFs(), logrus.New(), test.line, make(chan struct{}), ) if test.err { From 14ef6c240712eaa822e314a534709c805b5d2624 Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Sun, 6 Mar 2022 17:00:12 +0200 Subject: [PATCH 2/3] Test that --logformat is deprecated but still works --- cmd/root.go | 11 +++++++---- cmd/root_test.go | 30 ++++++++++++++++++++++++++++-- cmd/run_test.go | 12 +++--------- lib/testutils/logrus_hook.go | 12 ++++++++++++ 4 files changed, 50 insertions(+), 15 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 97d4fe97e72..342bbc327fc 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -89,7 +89,7 @@ type globalState struct { outMutex *sync.Mutex stdOut, stdErr *consoleWriter - stdIn *os.File + stdIn io.Reader signalNotify func(chan<- os.Signal, ...os.Signal) signalStop func(chan<- os.Signal) @@ -219,6 +219,12 @@ func newRootCommand(gs *globalState) *rootCommand { PersistentPreRunE: c.persistentPreRunE, } + rootCmd.PersistentFlags().AddFlagSet(rootCmdPersistentFlagSet(gs)) + rootCmd.SetArgs(gs.args[1:]) + rootCmd.SetOut(gs.stdOut) + rootCmd.SetErr(gs.stdErr) // TODO: use gs.logger.WriterLevel(logrus.ErrorLevel)? + rootCmd.SetIn(gs.stdIn) + loginCmd := getLoginCmd() loginCmd.AddCommand( getLoginCloudCommand(gs), @@ -230,10 +236,7 @@ func newRootCommand(gs *globalState) *rootCommand { getStatsCmd(gs), getStatusCmd(gs), getVersionCmd(gs), ) - rootCmd.PersistentFlags().AddFlagSet(rootCmdPersistentFlagSet(gs)) - rootCmd.SetArgs(gs.args[1:]) c.cmd = rootCmd - return c } diff --git a/cmd/root_test.go b/cmd/root_test.go index f8ecaeb6f6c..c620a8c5a1d 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -3,7 +3,6 @@ package cmd import ( "bytes" "context" - "os" "os/signal" "runtime" "sync" @@ -11,6 +10,7 @@ import ( "github.com/sirupsen/logrus" "github.com/spf13/afero" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.k6.io/k6/lib/testutils" ) @@ -63,7 +63,7 @@ func newGlobalTestState(t *testing.T) *globalTestState { outMutex: outMutex, stdOut: &consoleWriter{nil, ts.stdOut, false, outMutex, nil}, stdErr: &consoleWriter{nil, ts.stdErr, false, outMutex, nil}, - stdIn: os.Stdin, // TODO: spoof? + stdIn: new(bytes.Buffer), signalNotify: signal.Notify, signalStop: signal.Stop, logger: logger, @@ -71,3 +71,29 @@ func newGlobalTestState(t *testing.T) *globalTestState { } return ts } + +func TestDeprecatedOptionWarning(t *testing.T) { + t.Parallel() + + ts := newGlobalTestState(t) + ts.args = []string{"k6", "--logformat", "json", "run", "-"} + ts.stdIn = bytes.NewBuffer([]byte(` + console.log('foo'); + export default function() { console.log('bar'); }; + `)) + + root := newRootCommand(ts.globalState) + + require.NoError(t, root.cmd.Execute()) + + logMsgs := ts.loggerHook.Drain() + assert.True(t, testutils.LogContains(logMsgs, logrus.InfoLevel, "foo")) + assert.True(t, testutils.LogContains(logMsgs, logrus.InfoLevel, "bar")) + assert.Contains(t, ts.stdErr.String(), `"level":"info","msg":"foo","source":"console"`) + assert.Contains(t, ts.stdErr.String(), `"level":"info","msg":"bar","source":"console"`) + + // TODO: after we get rid of cobra, actually emit this message to stderr + // and, ideally, through the log, not just print it... + assert.False(t, testutils.LogContains(logMsgs, logrus.InfoLevel, "logformat")) + assert.Contains(t, ts.stdOut.String(), `--logformat has been deprecated`) +} diff --git a/cmd/run_test.go b/cmd/run_test.go index a11e9800de2..ba1af0cfc7f 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -30,9 +30,9 @@ import ( "path" "path/filepath" "runtime" - "strings" "testing" + "github.com/sirupsen/logrus" "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -41,6 +41,7 @@ import ( "go.k6.io/k6/errext/exitcodes" "go.k6.io/k6/js/common" "go.k6.io/k6/lib/fsext" + "go.k6.io/k6/lib/testutils" ) type mockWriter struct { @@ -212,14 +213,7 @@ func TestRunScriptErrorsAndAbort(t *testing.T) { } if tc.expLogOutput != "" { - var gotMsg bool - for _, entry := range testState.loggerHook.Drain() { - if strings.Contains(entry.Message, tc.expLogOutput) { - gotMsg = true - break - } - } - assert.True(t, gotMsg) + assert.True(t, testutils.LogContains(testState.loggerHook.Drain(), logrus.InfoLevel, tc.expLogOutput)) } }) } diff --git a/lib/testutils/logrus_hook.go b/lib/testutils/logrus_hook.go index 5c41d855639..2436e9ffb98 100644 --- a/lib/testutils/logrus_hook.go +++ b/lib/testutils/logrus_hook.go @@ -21,6 +21,7 @@ package testutils import ( + "strings" "sync" "github.com/sirupsen/logrus" @@ -57,3 +58,14 @@ func (smh *SimpleLogrusHook) Drain() []logrus.Entry { } var _ logrus.Hook = &SimpleLogrusHook{} + +// LogContains is a helper function that checks the provided list of log entries +// for a message matching the provided level and contents. +func LogContains(logEntries []logrus.Entry, expLevel logrus.Level, expContents string) bool { + for _, entry := range logEntries { + if entry.Level == expLevel && strings.Contains(entry.Message, expContents) { + return true + } + } + return false +} From d49634c76b9cabc355b79c167581534c85e753b7 Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Sun, 6 Mar 2022 17:09:40 +0200 Subject: [PATCH 3/3] Start with a colorful log if stderr is TTY, but also respect NO_COLOR This will ensure we have the same behavior as previous k6 version there's an error before setupLoggers() is executed, e.g. when parsing a wrong CLI flag. However, we will now also respect NO_COLOR and K6_NO_COLOR and disable it when they are specified. --- cmd/root.go | 70 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 342bbc327fc..de20f3724d1 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -31,6 +31,7 @@ import ( "os" "os/signal" "path/filepath" + "strconv" "strings" "sync" "time" @@ -112,7 +113,17 @@ func newGlobalState(ctx context.Context) *globalState { stdOut := &consoleWriter{os.Stdout, colorable.NewColorable(os.Stdout), stdoutTTY, outMutex, nil} stdErr := &consoleWriter{os.Stderr, colorable.NewColorable(os.Stderr), stderrTTY, outMutex, nil} - logger := getDefaultLogger(stdErr) + envVars := buildEnvMap(os.Environ()) + _, noColorsSet := envVars["NO_COLOR"] // even empty values disable colors + logger := &logrus.Logger{ + Out: stdErr, + Formatter: &logrus.TextFormatter{ + ForceColors: stderrTTY, + DisableColors: !stderrTTY || noColorsSet || envVars["K6_NO_COLOR"] != "", + }, + Hooks: make(logrus.LevelHooks), + Level: logrus.InfoLevel, + } confDir, err := os.UserConfigDir() if err != nil { @@ -120,34 +131,29 @@ func newGlobalState(ctx context.Context) *globalState { confDir = ".config" } - envVars := buildEnvMap(os.Environ()) defaultFlags := getDefaultFlags(confDir) return &globalState{ - ctx: ctx, - fs: afero.NewOsFs(), - getwd: os.Getwd, - args: append(make([]string, 0, len(os.Args)), os.Args...), // copy - envVars: envVars, - defaultFlags: defaultFlags, - flags: getFlags(defaultFlags, envVars), - outMutex: outMutex, - stdOut: stdOut, - stdErr: stdErr, - stdIn: os.Stdin, - signalNotify: signal.Notify, - signalStop: signal.Stop, - logger: logger, - fallbackLogger: getDefaultLogger(stdErr), // we may modify the other one - } -} - -func getDefaultLogger(out io.Writer) *logrus.Logger { - return &logrus.Logger{ - Out: out, - Formatter: new(logrus.TextFormatter), - Hooks: make(logrus.LevelHooks), - Level: logrus.InfoLevel, + ctx: ctx, + fs: afero.NewOsFs(), + getwd: os.Getwd, + args: append(make([]string, 0, len(os.Args)), os.Args...), // copy + envVars: envVars, + defaultFlags: defaultFlags, + flags: getFlags(defaultFlags, envVars), + outMutex: outMutex, + stdOut: stdOut, + stdErr: stdErr, + stdIn: os.Stdin, + signalNotify: signal.Notify, + signalStop: signal.Stop, + logger: logger, + fallbackLogger: &logrus.Logger{ // we may modify the other one + Out: stdErr, + Formatter: new(logrus.TextFormatter), // no fancy formatting here + Hooks: make(logrus.LevelHooks), + Level: logrus.InfoLevel, + }, } } @@ -177,6 +183,14 @@ func getFlags(defaultFlags globalFlags, env map[string]string) globalFlags { if val, ok := env["K6_LOG_FORMAT"]; ok { result.logFormat = val } + if env["K6_NO_COLOR"] != "" { + result.noColor = true + } + // Support https://no-color.org/, even an empty value should disable the + // color output from k6. + if _, ok := env["NO_COLOR"]; ok { + result.noColor = true + } return result } @@ -341,11 +355,13 @@ func rootCmdPersistentFlagSet(gs *globalState) *pflag.FlagSet { flags.Lookup("config").DefValue = gs.defaultFlags.configFilePath must(cobra.MarkFlagFilename(flags, "config")) + flags.BoolVar(&gs.flags.noColor, "no-color", gs.flags.noColor, "disable colored output") + flags.Lookup("no-color").DefValue = strconv.FormatBool(gs.defaultFlags.noColor) + // TODO: support configuring these through environment variables as well? // either with croconf or through the hack above... flags.BoolVarP(&gs.flags.verbose, "verbose", "v", gs.defaultFlags.verbose, "enable verbose logging") flags.BoolVarP(&gs.flags.quiet, "quiet", "q", gs.defaultFlags.quiet, "disable progress updates") - flags.BoolVar(&gs.flags.noColor, "no-color", gs.defaultFlags.noColor, "disable colored output") flags.StringVarP(&gs.flags.address, "address", "a", gs.defaultFlags.address, "address for the REST API server") return flags