From 8e8da9609ddbf5371dedb7a8f78818f34db815c2 Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Sun, 6 Mar 2022 23:01:56 +0200 Subject: [PATCH 01/10] Refactor test loading by moving it to a new helper function --- cmd/archive.go | 60 ++--------- cmd/cloud.go | 70 +++---------- cmd/common.go | 24 ----- cmd/config.go | 10 ++ cmd/inspect.go | 67 +++--------- cmd/root.go | 4 +- cmd/run.go | 109 +++----------------- cmd/runtime_options_test.go | 59 +++++------ cmd/test_load.go | 199 ++++++++++++++++++++++++++++++++++++ 9 files changed, 297 insertions(+), 305 deletions(-) create mode 100644 cmd/test_load.go diff --git a/cmd/archive.go b/cmd/archive.go index ae60d9d0cda..bd57e53468a 100644 --- a/cmd/archive.go +++ b/cmd/archive.go @@ -23,13 +23,9 @@ package cmd import ( "github.com/spf13/cobra" "github.com/spf13/pflag" - - "go.k6.io/k6/errext" - "go.k6.io/k6/errext/exitcodes" - "go.k6.io/k6/lib/metrics" ) -func getArchiveCmd(globalState *globalState) *cobra.Command { // nolint: funlen +func getArchiveCmd(gs *globalState) *cobra.Command { archiveOut := "archive.tar" // archiveCmd represents the archive command archiveCmd := &cobra.Command{ @@ -46,60 +42,24 @@ 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(globalState, args[0]) - if err != nil { - return err - } - - runtimeOptions, err := getRuntimeOptions(cmd.Flags(), globalState.envVars) - if err != nil { - return err - } - - registry := metrics.NewRegistry() - builtinMetrics := metrics.RegisterBuiltinMetrics(registry) - r, err := newRunner( - globalState.logger, src, globalState.flags.runType, - filesystems, runtimeOptions, builtinMetrics, registry, - ) - if err != nil { - return err - } - - cliOpts, err := getOptions(cmd.Flags()) - if err != nil { - return err - } - conf, err := getConsolidatedConfig(globalState, Config{Options: cliOpts}, r.GetOptions()) - if err != nil { - return err - } - - // Parse the thresholds, only if the --no-threshold flag is not set. - // If parsing the threshold expressions failed, consider it as an - // invalid configuration error. - if !runtimeOptions.NoThresholds.Bool { - for _, thresholds := range conf.Options.Thresholds { - err = thresholds.Parse() - if err != nil { - return errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) - } - } - } - - _, err = deriveAndValidateConfig(conf, r.IsExecutable, globalState.logger) + test, err := loadTest(gs, cmd, args, getPartialConfig) if err != nil { return err } - err = r.SetOptions(conf.Options) + // It's important to NOT set the derived options back to the runner + // here, only the consolidated ones. Otherwise, if the script used + // an execution shortcut option (e.g. `iterations` or `duration`), + // we will have multiple conflicting execution options since the + // derivation will set `scenarios` as well. + err = test.initRunner.SetOptions(test.consolidatedConfig.Options) if err != nil { return err } // Archive. - arc := r.MakeArchive() - f, err := globalState.fs.Create(archiveOut) + arc := test.initRunner.MakeArchive() + f, err := gs.fs.Create(archiveOut) if err != nil { return err } diff --git a/cmd/cloud.go b/cmd/cloud.go index bbb0d718770..cd31d4a80b7 100644 --- a/cmd/cloud.go +++ b/cmd/cloud.go @@ -42,7 +42,6 @@ import ( "go.k6.io/k6/errext/exitcodes" "go.k6.io/k6/lib" "go.k6.io/k6/lib/consts" - "go.k6.io/k6/lib/metrics" "go.k6.io/k6/ui/pb" ) @@ -91,56 +90,23 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth RunE: func(cmd *cobra.Command, args []string) error { printBanner(globalState) - logger := globalState.logger progressBar := pb.New( pb.WithConstLeft("Init"), - pb.WithConstProgress(0, "Parsing script"), + pb.WithConstProgress(0, "Loading test script..."), ) printBar(globalState, progressBar) - // Runner - filename := args[0] - src, filesystems, err := readSource(globalState, filename) + test, err := loadTest(globalState, cmd, args, getPartialConfig) if err != nil { return err } - runtimeOptions, err := getRuntimeOptions(cmd.Flags(), globalState.envVars) - if err != nil { - return err - } - - modifyAndPrintBar(globalState, progressBar, pb.WithConstProgress(0, "Getting script options")) - registry := metrics.NewRegistry() - builtinMetrics := metrics.RegisterBuiltinMetrics(registry) - r, err := newRunner(logger, src, globalState.flags.runType, filesystems, runtimeOptions, builtinMetrics, registry) - if err != nil { - return err - } - - modifyAndPrintBar(globalState, progressBar, pb.WithConstProgress(0, "Consolidating options")) - cliOpts, err := getOptions(cmd.Flags()) - if err != nil { - return err - } - conf, err := getConsolidatedConfig(globalState, Config{Options: cliOpts}, r.GetOptions()) - if err != nil { - return err - } - - // Parse the thresholds, only if the --no-threshold flag is not set. - // If parsing the threshold expressions failed, consider it as an - // invalid configuration error. - if !runtimeOptions.NoThresholds.Bool { - for _, thresholds := range conf.Options.Thresholds { - err = thresholds.Parse() - if err != nil { - return errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) - } - } - } - - derivedConf, err := deriveAndValidateConfig(conf, r.IsExecutable, logger) + // It's important to NOT set the derived options back to the runner + // here, only the consolidated ones. Otherwise, if the script used + // an execution shortcut option (e.g. `iterations` or `duration`), + // we will have multiple conflicting execution options since the + // derivation will set `scenarios` as well. + err = test.initRunner.SetOptions(test.consolidatedConfig.Options) if err != nil { return err } @@ -149,13 +115,9 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth // TODO: validate for externally controlled executor (i.e. executors that aren't distributable) // TODO: move those validations to a separate function and reuse validateConfig()? - err = r.SetOptions(conf.Options) - if err != nil { - return err - } + modifyAndPrintBar(globalState, progressBar, pb.WithConstProgress(0, "Building the archive...")) + arc := test.initRunner.MakeArchive() - 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 // done, as the idea of options.ext is that they are extensible without touching k6. But in @@ -174,7 +136,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"], globalState.envVars, "", arc.Options.External) + test.derivedConfig.Collectors["cloud"], globalState.envVars, "", arc.Options.External) if err != nil { return err } @@ -205,12 +167,14 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth name := cloudConfig.Name.String if !cloudConfig.Name.Valid || cloudConfig.Name.String == "" { - name = filepath.Base(filename) + name = filepath.Base(test.testPath) } globalCtx, globalCancel := context.WithCancel(globalState.ctx) defer globalCancel() + logger := globalState.logger + // Start cloud test run modifyAndPrintBar(globalState, progressBar, pb.WithConstProgress(0, "Validating script options")) client := cloudapi.NewClient( @@ -248,14 +212,14 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth os.Exit(int(exitcodes.ExternalAbort)) }() - et, err := lib.NewExecutionTuple(derivedConf.ExecutionSegment, derivedConf.ExecutionSegmentSequence) + et, err := lib.NewExecutionTuple(test.derivedConfig.ExecutionSegment, test.derivedConfig.ExecutionSegmentSequence) if err != nil { return err } testURL := cloudapi.URLForResults(refID, cloudConfig) - executionPlan := derivedConf.Scenarios.GetFullExecutionRequirements(et) + executionPlan := test.derivedConfig.Scenarios.GetFullExecutionRequirements(et) printExecutionDescription( - globalState, "cloud", filename, testURL, derivedConf, et, executionPlan, nil, + globalState, "cloud", test.testPath, testURL, test.derivedConfig, et, executionPlan, nil, ) modifyAndPrintBar( diff --git a/cmd/common.go b/cmd/common.go index 43af05b40db..fba5a02d76d 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -21,17 +21,13 @@ package cmd import ( - "archive/tar" - "bytes" "fmt" - "github.com/spf13/afero" "github.com/spf13/cobra" "github.com/spf13/pflag" "gopkg.in/guregu/null.v3" "go.k6.io/k6/lib/types" - "go.k6.io/k6/loader" ) // Panic if the given error is not nil. @@ -86,26 +82,6 @@ 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(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(globalState.fs) - src, err := loader.ReadSource(globalState.logger, filename, pwd, filesystems, globalState.stdIn) - return src, filesystems, err -} - -func detectType(data []byte) string { - if _, err := tar.NewReader(bytes.NewReader(data)).Next(); err == nil { - return typeArchive - } - return typeJS -} - func printToStdout(gs *globalState, s string) { if _, err := fmt.Fprint(gs.stdOut, s); err != nil { gs.logger.Errorf("could not print '%s' to stdout: %s", s, err.Error()) diff --git a/cmd/config.go b/cmd/config.go index 6a4c8fae5cd..df1243a7665 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -91,6 +91,16 @@ func (c Config) Apply(cfg Config) Config { return c } +// Returns a Config but only parses the Options inside. +func getPartialConfig(flags *pflag.FlagSet) (Config, error) { + opts, err := getOptions(flags) + if err != nil { + return Config{}, err + } + + return Config{Options: opts}, nil +} + // Gets configuration from CLI flags. func getConfig(flags *pflag.FlagSet) (Config, error) { opts, err := getOptions(flags) diff --git a/cmd/inspect.go b/cmd/inspect.go index 2acc5e5e0c1..8eb421c2832 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -21,19 +21,15 @@ package cmd import ( - "bytes" "encoding/json" - "fmt" "github.com/spf13/cobra" - "go.k6.io/k6/js" "go.k6.io/k6/lib" - "go.k6.io/k6/lib/metrics" "go.k6.io/k6/lib/types" ) -func getInspectCmd(globalState *globalState) *cobra.Command { +func getInspectCmd(gs *globalState) *cobra.Command { var addExecReqs bool // inspectCmd represents the inspect command @@ -43,44 +39,18 @@ func getInspectCmd(globalState *globalState) *cobra.Command { Long: `Inspect a script or archive.`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - src, filesystems, err := readSource(globalState, args[0]) + test, err := loadTest(gs, cmd, args, nil) if err != nil { return err } - runtimeOptions, err := getRuntimeOptions(cmd.Flags(), globalState.envVars) - if err != nil { - return err - } - registry := metrics.NewRegistry() - - var b *js.Bundle - typ := globalState.flags.runType - if typ == "" { - typ = detectType(src.Data) - } - switch typ { - // this is an exhaustive list - case typeArchive: - var arc *lib.Archive - arc, err = lib.ReadArchive(bytes.NewBuffer(src.Data)) - if err != nil { - return err - } - b, err = js.NewBundleFromArchive(globalState.logger, arc, runtimeOptions, registry) - - case typeJS: - b, err = js.NewBundle(globalState.logger, src, filesystems, runtimeOptions, registry) - } - if err != nil { - return err - } - - // ATM, output can take 2 forms: standard (equal to lib.Options struct) and extended, with additional fields. - inspectOutput := interface{}(b.Options) + // At the moment, `k6 inspect` output can take 2 forms: standard + // (equal to the lib.Options struct) and extended, with additional + // fields with execution requirements. + inspectOutput := interface{}(test.initRunner.GetOptions()) if addExecReqs { - inspectOutput, err = addExecRequirements(globalState, b) + inspectOutput, err = addExecRequirements(gs, cmd, test) if err != nil { return err } @@ -90,7 +60,7 @@ func getInspectCmd(globalState *globalState) *cobra.Command { if err != nil { return err } - fmt.Println(string(data)) //nolint:forbidigo // yes we want to just print it + printToStdout(gs, string(data)) return nil }, @@ -98,8 +68,8 @@ func getInspectCmd(globalState *globalState) *cobra.Command { inspectCmd.Flags().SortFlags = false inspectCmd.Flags().AddFlagSet(runtimeOptionFlagSet(false)) - inspectCmd.Flags().StringVarP(&globalState.flags.runType, "type", "t", - globalState.flags.runType, "override file `type`, \"js\" or \"archive\"") + inspectCmd.Flags().StringVarP(&gs.flags.testType, "type", "t", + gs.flags.testType, "override file `type`, \"js\" or \"archive\"") inspectCmd.Flags().BoolVar(&addExecReqs, "execution-requirements", false, @@ -108,23 +78,18 @@ func getInspectCmd(globalState *globalState) *cobra.Command { return inspectCmd } -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, gs.logger) - if err != nil { +func addExecRequirements(gs *globalState, cmd *cobra.Command, test *loadedTest) (interface{}, error) { + // we don't actually support CLI flags here, so we pass nil as the getter + if err := test.consolidateDeriveAndValidateConfig(gs, cmd, nil); err != nil { return nil, err } - et, err := lib.NewExecutionTuple(conf.ExecutionSegment, conf.ExecutionSegmentSequence) + et, err := lib.NewExecutionTuple(test.derivedConfig.ExecutionSegment, test.derivedConfig.ExecutionSegmentSequence) if err != nil { return nil, err } - executionPlan := conf.Scenarios.GetFullExecutionRequirements(et) + executionPlan := test.derivedConfig.Scenarios.GetFullExecutionRequirements(et) duration, _ := lib.GetEndOffset(executionPlan) return struct { @@ -132,7 +97,7 @@ func addExecRequirements(gs *globalState, b *js.Bundle) (interface{}, error) { TotalDuration types.NullDuration `json:"totalDuration"` MaxVUs uint64 `json:"maxVUs"` }{ - conf.Options, + test.derivedConfig.Options, types.NewNullDuration(duration, true), lib.GetMaxPossibleVUs(executionPlan), }, nil diff --git a/cmd/root.go b/cmd/root.go index de20f3724d1..9ceed327180 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -56,7 +56,7 @@ const ( // globalFlags contains global config values that apply for all k6 sub-commands. type globalFlags struct { configFilePath string - runType string + testType string // TODO: move to RuntimeOptions, it's not trully global quiet bool noColor bool address string @@ -175,7 +175,7 @@ func getFlags(defaultFlags globalFlags, env map[string]string) globalFlags { result.configFilePath = val } if val, ok := env["K6_TYPE"]; ok { - result.runType = val + result.testType = val } if val, ok := env["K6_LOG_OUTPUT"]; ok { result.logOutput = val diff --git a/cmd/run.go b/cmd/run.go index 6404a0c203d..0779af43b74 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -34,7 +34,6 @@ import ( "syscall" "time" - "github.com/sirupsen/logrus" "github.com/spf13/afero" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -44,20 +43,12 @@ import ( "go.k6.io/k6/core/local" "go.k6.io/k6/errext" "go.k6.io/k6/errext/exitcodes" - "go.k6.io/k6/js" "go.k6.io/k6/js/common" "go.k6.io/k6/lib" "go.k6.io/k6/lib/consts" - "go.k6.io/k6/lib/metrics" - "go.k6.io/k6/loader" "go.k6.io/k6/ui/pb" ) -const ( - typeJS = "js" - typeArchive = "archive" -) - //nolint:funlen,gocognit,gocyclo,cyclop func getRunCmd(globalState *globalState) *cobra.Command { // runCmd represents the run command. @@ -90,59 +81,14 @@ a commandline interface for interacting with it.`, RunE: func(cmd *cobra.Command, args []string) error { printBanner(globalState) - logger := globalState.logger - logger.Debug("Initializing the runner...") - - // Create the Runner. - src, filesystems, err := readSource(globalState, args[0]) + test, err := loadTest(globalState, cmd, args, getConfig) if err != nil { return err } - runtimeOptions, err := getRuntimeOptions(cmd.Flags(), globalState.envVars) - if err != nil { - return err - } - - registry := metrics.NewRegistry() - builtinMetrics := metrics.RegisterBuiltinMetrics(registry) - initRunner, err := newRunner( - logger, src, globalState.flags.runType, filesystems, runtimeOptions, builtinMetrics, registry, - ) - if err != nil { - return common.UnwrapGojaInterruptedError(err) - } - - logger.Debug("Getting the script options...") - - cliConf, err := getConfig(cmd.Flags()) - if err != nil { - return err - } - conf, err := getConsolidatedConfig(globalState, cliConf, initRunner.GetOptions()) - if err != nil { - return err - } - - // Parse the thresholds, only if the --no-threshold flag is not set. - // If parsing the threshold expressions failed, consider it as an - // invalid configuration error. - if !runtimeOptions.NoThresholds.Bool { - for _, thresholds := range conf.Options.Thresholds { - err = thresholds.Parse() - if err != nil { - return errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) - } - } - } - - conf, err = deriveAndValidateConfig(conf, initRunner.IsExecutable, logger) - if err != nil { - return err - } - - // Write options back to the runner too. - if err = initRunner.SetOptions(conf.Options); err != nil { + // Write the full consolidated *and derived* options back to the Runner. + conf := test.derivedConfig + if err = test.initRunner.SetOptions(conf.Options); err != nil { return err } @@ -163,9 +109,10 @@ a commandline interface for interacting with it.`, runCtx, runCancel := context.WithCancel(lingerCtx) defer runCancel() + logger := globalState.logger // Create a local execution scheduler wrapping the runner. logger.Debug("Initializing the execution scheduler...") - execScheduler, err := local.NewExecutionScheduler(initRunner, logger) + execScheduler, err := local.NewExecutionScheduler(test.initRunner, logger) if err != nil { return err } @@ -190,14 +137,17 @@ a commandline interface for interacting with it.`, // Create all outputs. executionPlan := execScheduler.GetExecutionPlan() - outputs, err := createOutputs(globalState, src, conf, runtimeOptions, executionPlan) + outputs, err := createOutputs(globalState, test.source, conf, test.runtimeOptions, executionPlan) if err != nil { return err } // Create the engine. initBar.Modify(pb.WithConstProgress(0, "Init engine")) - engine, err := core.NewEngine(execScheduler, conf.Options, runtimeOptions, outputs, logger, builtinMetrics) + engine, err := core.NewEngine( + execScheduler, conf.Options, test.runtimeOptions, + outputs, logger, test.builtInMetrics, + ) if err != nil { return err } @@ -302,8 +252,8 @@ a commandline interface for interacting with it.`, } // Handle the end-of-test summary. - if !runtimeOptions.NoSummary.Bool { - summaryResult, err := initRunner.HandleSummary(globalCtx, &lib.Summary{ + if !test.runtimeOptions.NoSummary.Bool { + summaryResult, err := test.initRunner.HandleSummary(globalCtx, &lib.Summary{ Metrics: engine.Metrics, RootGroup: engine.ExecutionScheduler.GetRunner().GetDefaultGroup(), TestRunDuration: executionState.GetCurrentTestRunDuration(), @@ -399,41 +349,12 @@ func runCmdFlagSet(globalState *globalState) *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(&globalState.flags.runType, "type", "t", - globalState.flags.runType, "override file `type`, \"js\" or \"archive\"") + flags.StringVarP(&globalState.flags.testType, "type", "t", + globalState.flags.testType, "override file `type`, \"js\" or \"archive\"") flags.Lookup("type").DefValue = "" return flags } -// Creates a new runner. -func newRunner( - logger *logrus.Logger, src *loader.SourceData, typ string, filesystems map[string]afero.Fs, rtOpts lib.RuntimeOptions, - builtinMetrics *metrics.BuiltinMetrics, registry *metrics.Registry, -) (runner lib.Runner, err error) { - switch typ { - case "": - runner, err = newRunner(logger, src, detectType(src.Data), filesystems, rtOpts, builtinMetrics, registry) - case typeJS: - runner, err = js.New(logger, src, filesystems, rtOpts, builtinMetrics, registry) - case typeArchive: - var arc *lib.Archive - arc, err = lib.ReadArchive(bytes.NewReader(src.Data)) - if err != nil { - return nil, err - } - switch arc.Type { - case typeJS: - runner, err = js.NewFromArchive(logger, arc, rtOpts, builtinMetrics, registry) - default: - return nil, fmt.Errorf("archive requests unsupported runner: %s", arc.Type) - } - default: - return nil, fmt.Errorf("unknown -t/--type: %s", typ) - } - - return runner, err -} - func handleSummaryResult(fs afero.Fs, stdOut, stdErr io.Writer, result map[string]io.Reader) error { var errs []error diff --git a/cmd/runtime_options_test.go b/cmd/runtime_options_test.go index db910c2a0fd..65107db7d25 100644 --- a/cmd/runtime_options_test.go +++ b/cmd/runtime_options_test.go @@ -33,7 +33,6 @@ import ( "go.k6.io/k6/lib" "go.k6.io/k6/lib/metrics" - "go.k6.io/k6/lib/testutils" "go.k6.io/k6/loader" ) @@ -78,44 +77,42 @@ func testRuntimeOptionsCase(t *testing.T, tc runtimeOptionsTestCase) { fs := afero.NewMemMapFs() require.NoError(t, afero.WriteFile(fs, "/script.js", jsCode.Bytes(), 0o644)) + + ts := newGlobalTestState(t) // TODO: move upwards, make this into an almost full integration test registry := metrics.NewRegistry() - builtinMetrics := metrics.RegisterBuiltinMetrics(registry) - runner, err := newRunner( - testutils.NewLogger(t), - &loader.SourceData{Data: jsCode.Bytes(), URL: &url.URL{Path: "/script.js", Scheme: "file"}}, - typeJS, - map[string]afero.Fs{"file": fs}, - rtOpts, - builtinMetrics, - registry, - ) - require.NoError(t, err) + test := &loadedTest{ + testPath: "script.js", + source: &loader.SourceData{Data: jsCode.Bytes(), URL: &url.URL{Path: "/script.js", Scheme: "file"}}, + fileSystems: map[string]afero.Fs{"file": fs}, + runtimeOptions: rtOpts, + metricsRegistry: registry, + builtInMetrics: metrics.RegisterBuiltinMetrics(registry), + } + + require.NoError(t, test.initializeFirstRunner(ts.globalState)) - archive := runner.MakeArchive() + archive := test.initRunner.MakeArchive() archiveBuf := &bytes.Buffer{} require.NoError(t, archive.Write(archiveBuf)) - getRunnerErr := func(rtOpts lib.RuntimeOptions) (lib.Runner, error) { - return newRunner( - testutils.NewLogger(t), - &loader.SourceData{ - Data: archiveBuf.Bytes(), - URL: &url.URL{Path: "/script.js"}, - }, - typeArchive, - nil, - rtOpts, - builtinMetrics, - registry, - ) + getRunnerErr := func(rtOpts lib.RuntimeOptions) *loadedTest { + return &loadedTest{ + testPath: "script.tar", + source: &loader.SourceData{Data: archiveBuf.Bytes(), URL: &url.URL{Path: "/script.tar", Scheme: "file"}}, + fileSystems: map[string]afero.Fs{"file": fs}, + runtimeOptions: rtOpts, + metricsRegistry: registry, + builtInMetrics: metrics.RegisterBuiltinMetrics(registry), + } } - _, err = getRunnerErr(lib.RuntimeOptions{}) - require.NoError(t, err) + archTest := getRunnerErr(lib.RuntimeOptions{}) + require.NoError(t, archTest.initializeFirstRunner(ts.globalState)) + for key, val := range tc.expRTOpts.Env { - r, err := getRunnerErr(lib.RuntimeOptions{Env: map[string]string{key: "almost " + val}}) - assert.NoError(t, err) - assert.Equal(t, r.MakeArchive().Env[key], "almost "+val) + archTest = getRunnerErr(lib.RuntimeOptions{Env: map[string]string{key: "almost " + val}}) + require.NoError(t, archTest.initializeFirstRunner(ts.globalState)) + assert.Equal(t, archTest.initRunner.MakeArchive().Env[key], "almost "+val) } } diff --git a/cmd/test_load.go b/cmd/test_load.go new file mode 100644 index 00000000000..5e51fae4b25 --- /dev/null +++ b/cmd/test_load.go @@ -0,0 +1,199 @@ +package cmd + +import ( + "archive/tar" + "bytes" + "fmt" + + "github.com/spf13/afero" + "github.com/spf13/cobra" + "github.com/spf13/pflag" + "go.k6.io/k6/errext" + "go.k6.io/k6/errext/exitcodes" + "go.k6.io/k6/js" + "go.k6.io/k6/lib" + "go.k6.io/k6/lib/metrics" + "go.k6.io/k6/loader" +) + +const ( + testTypeJS = "js" + testTypeArchive = "archive" +) + +type loadedTest struct { + testPath string // contains the raw string the user supplied + source *loader.SourceData + fileSystems map[string]afero.Fs + runtimeOptions lib.RuntimeOptions + metricsRegistry *metrics.Registry + builtInMetrics *metrics.BuiltinMetrics + initRunner lib.Runner // TODO: rename to something more appropriate + + // Only set if cliConfigGetter is supplied to loadTest() or if + // consolidateDeriveAndValidateConfig() is manually called. + consolidatedConfig Config + derivedConfig Config +} + +func loadTest( + gs *globalState, cmd *cobra.Command, args []string, + // supply this if you want the test config consolidated and validated + cliConfigGetter func(flags *pflag.FlagSet) (Config, error), // TODO: obviate +) (*loadedTest, error) { + if len(args) < 1 { + return nil, fmt.Errorf("k6 needs at least one argument to load the test") + } + + testPath := args[0] + gs.logger.Debugf("Resolving and reading test '%s'...", testPath) + src, fileSystems, err := readSource(gs, testPath) + if err != nil { + return nil, err + } + resolvedPath := src.URL.String() + gs.logger.Debugf("'%s' resolved to '%s' and successfully loaded %d bytes!", testPath, resolvedPath, len(src.Data)) + + gs.logger.Debugf("Gathering k6 runtime options...") + runtimeOptions, err := getRuntimeOptions(cmd.Flags(), gs.envVars) + if err != nil { + return nil, err + } + + registry := metrics.NewRegistry() + test := &loadedTest{ + testPath: testPath, + source: src, + fileSystems: fileSystems, + runtimeOptions: runtimeOptions, + metricsRegistry: registry, + builtInMetrics: metrics.RegisterBuiltinMetrics(registry), + } + + gs.logger.Debugf("Initializing k6 runner for '%s' (%s)...", testPath, resolvedPath) + if err := test.initializeFirstRunner(gs); err != nil { + return nil, fmt.Errorf("could not initialize '%s': %w", testPath, err) + } + gs.logger.Debug("Runner successfully initialized!") + + if cliConfigGetter != nil { + if err := test.consolidateDeriveAndValidateConfig(gs, cmd, cliConfigGetter); err != nil { + return nil, err + } + } + + return test, nil +} + +func (lt *loadedTest) initializeFirstRunner(gs *globalState) error { + testPath := lt.source.URL.String() + logger := gs.logger.WithField("test_path", testPath) + + testType := gs.flags.testType + if testType == "" { + logger.Debug("Detecting test type for...") + testType = detectTestType(lt.source.Data) + } + + switch testType { + case testTypeJS: + logger.Debug("Trying to load as a JS test...") + runner, err := js.New( + gs.logger, lt.source, lt.fileSystems, lt.runtimeOptions, lt.builtInMetrics, lt.metricsRegistry, + ) + // TODO: should we use common.UnwrapGojaInterruptedError() here? + if err != nil { + return fmt.Errorf("could not load JS test '%s': %w", testPath, err) + } + lt.initRunner = runner + return nil + + case testTypeArchive: + logger.Debug("Trying to load test as an archive bundle...") + + var arc *lib.Archive + arc, err := lib.ReadArchive(bytes.NewReader(lt.source.Data)) + if err != nil { + return fmt.Errorf("could not load test archive bundle '%s': %w", testPath, err) + } + logger.Debugf("Loaded test as an archive bundle with type '%s'!", arc.Type) + + switch arc.Type { + case testTypeJS: + logger.Debug("Evaluating JS from archive bundle...") + lt.initRunner, err = js.NewFromArchive(gs.logger, arc, lt.runtimeOptions, lt.builtInMetrics, lt.metricsRegistry) + if err != nil { + return fmt.Errorf("could not load JS from test archive bundle '%s': %w", testPath, err) + } + return nil + default: + return fmt.Errorf("archive '%s' has an unsupported test type '%s'", testPath, arc.Type) + } + default: + return fmt.Errorf("unknown or unspecified test type '%s' for '%s'", testType, testPath) + } +} + +// readSource is a small wrapper around loader.ReadSource returning +// result of the load and filesystems map +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(globalState.fs) + src, err := loader.ReadSource(globalState.logger, filename, pwd, filesystems, globalState.stdIn) + return src, filesystems, err +} + +func detectTestType(data []byte) string { + if _, err := tar.NewReader(bytes.NewReader(data)).Next(); err == nil { + return testTypeArchive + } + return testTypeJS +} + +func (lt *loadedTest) consolidateDeriveAndValidateConfig( + gs *globalState, cmd *cobra.Command, + cliConfGetter func(flags *pflag.FlagSet) (Config, error), // TODO: obviate +) error { + var cliConfig Config + if cliConfGetter != nil { + gs.logger.Debug("Parsing CLI flags...") + var err error + cliConfig, err = cliConfGetter(cmd.Flags()) + if err != nil { + return err + } + } + + gs.logger.Debug("Consolidating config layers...") + consolidatedConfig, err := getConsolidatedConfig(gs, cliConfig, lt.initRunner.GetOptions()) + if err != nil { + return err + } + + gs.logger.Debug("Parsing thresholds and validating config...") + // Parse the thresholds, only if the --no-threshold flag is not set. + // If parsing the threshold expressions failed, consider it as an + // invalid configuration error. + if !lt.runtimeOptions.NoThresholds.Bool { + for _, thresholds := range consolidatedConfig.Options.Thresholds { + err = thresholds.Parse() + if err != nil { + return errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) + } + } + } + + derivedConfig, err := deriveAndValidateConfig(consolidatedConfig, lt.initRunner.IsExecutable, gs.logger) + if err != nil { + return err + } + + lt.consolidatedConfig = consolidatedConfig + lt.derivedConfig = derivedConfig + + return nil +} From 66d2f1f7d99fb47e3b3b5834dda06cecc7d2fe52 Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Sun, 6 Mar 2022 23:36:58 +0200 Subject: [PATCH 02/10] Refactor abort signal handling into a separate helper --- cmd/cloud.go | 19 ++++++++----------- cmd/common.go | 36 ++++++++++++++++++++++++++++++++++++ cmd/run.go | 18 ++++++------------ 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/cmd/cloud.go b/cmd/cloud.go index cd31d4a80b7..b51df6df3a2 100644 --- a/cmd/cloud.go +++ b/cmd/cloud.go @@ -30,7 +30,6 @@ import ( "path/filepath" "strconv" "sync" - "syscall" "time" "github.com/fatih/color" @@ -190,13 +189,10 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth } // Trap Interrupts, SIGINTs and SIGTERMs. - sigC := make(chan os.Signal, 2) - globalState.signalNotify(sigC, os.Interrupt, syscall.SIGINT, syscall.SIGTERM) - defer globalState.signalStop(sigC) - go func() { - sig := <-sigC + gracefulStop := func(sig os.Signal) { logger.WithField("sig", sig).Print("Stopping cloud test run in response to signal...") - // Do this in a separate goroutine so that if it blocks the second signal can stop the execution + // Do this in a separate goroutine so that if it blocks, the + // second signal can still abort the process execution. go func() { stopErr := client.StopCloudTestRun(refID) if stopErr != nil { @@ -206,11 +202,12 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth } globalCancel() }() - - sig = <-sigC + } + hardStop := func(sig os.Signal) { logger.WithField("sig", sig).Error("Aborting k6 in response to signal, we won't wait for the test to end.") - os.Exit(int(exitcodes.ExternalAbort)) - }() + } + stopSignalHandling := handleTestAbortSignals(globalState, gracefulStop, hardStop) + defer stopSignalHandling() et, err := lib.NewExecutionTuple(test.derivedConfig.ExecutionSegment, test.derivedConfig.ExecutionSegmentSequence) if err != nil { diff --git a/cmd/common.go b/cmd/common.go index fba5a02d76d..5b83ed53584 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -22,11 +22,14 @@ package cmd import ( "fmt" + "os" + "syscall" "github.com/spf13/cobra" "github.com/spf13/pflag" "gopkg.in/guregu/null.v3" + "go.k6.io/k6/errext/exitcodes" "go.k6.io/k6/lib/types" ) @@ -87,3 +90,36 @@ func printToStdout(gs *globalState, s string) { gs.logger.Errorf("could not print '%s' to stdout: %s", s, err.Error()) } } + +// Trap Interrupts, SIGINTs and SIGTERMs and call the given. +func handleTestAbortSignals(gs *globalState, firstHandler, secondHandler func(os.Signal)) (stop func()) { + sigC := make(chan os.Signal, 2) + done := make(chan struct{}) + gs.signalNotify(sigC, os.Interrupt, syscall.SIGINT, syscall.SIGTERM) + + go func() { + select { + case sig := <-sigC: + firstHandler(sig) + case <-done: + return + } + + select { + case sig := <-sigC: + if secondHandler != nil { + secondHandler(sig) + } + // If we get a second signal, we immediately exit, so something like + // https://github.com/k6io/k6/issues/971 never happens again + os.Exit(int(exitcodes.ExternalAbort)) + case <-done: + return + } + }() + + return func() { + close(done) + gs.signalStop(sigC) + } +} diff --git a/cmd/run.go b/cmd/run.go index 0779af43b74..e3b58cd5265 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -31,7 +31,6 @@ import ( "os" "runtime" "sync" - "syscall" "time" "github.com/spf13/afero" @@ -182,21 +181,16 @@ a commandline interface for interacting with it.`, ) // Trap Interrupts, SIGINTs and SIGTERMs. - sigC := make(chan os.Signal, 2) - globalState.signalNotify(sigC, os.Interrupt, syscall.SIGINT, syscall.SIGTERM) - defer globalState.signalStop(sigC) - go func() { - sig := <-sigC + gracefulStop := func(sig os.Signal) { logger.WithField("sig", sig).Debug("Stopping k6 in response to signal...") lingerCancel() // stop the test run, metric processing is cancelled below - - // If we get a second signal, we immediately exit, so something like - // https://github.com/k6io/k6/issues/971 never happens again - sig = <-sigC + } + hardStop := func(sig os.Signal) { logger.WithField("sig", sig).Error("Aborting k6 in response to signal") globalCancel() // not that it matters, given the following command... - os.Exit(int(exitcodes.ExternalAbort)) - }() + } + stopSignalHandling := handleTestAbortSignals(globalState, gracefulStop, hardStop) + defer stopSignalHandling() // Initialize the engine initBar.Modify(pb.WithConstProgress(0, "Init VUs...")) From 37ade417f22c740edaf15473569ea0517614e95e Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Sun, 6 Mar 2022 23:50:51 +0200 Subject: [PATCH 03/10] Move K6_TYPE / --type / -t out of the globalFlags to RuntimeOptions This actually also fixes a minor bug where the CLI flag was available in `k6 run` and `k6 inspect`, but it wasn't available in `k6 cloud` and `k6 archive`. --- cmd/inspect.go | 2 -- cmd/root.go | 4 ---- cmd/run.go | 15 ++------------- cmd/runtime_options.go | 12 ++++++++---- cmd/test_load.go | 2 +- lib/runtime_options.go | 2 ++ 6 files changed, 13 insertions(+), 24 deletions(-) diff --git a/cmd/inspect.go b/cmd/inspect.go index 8eb421c2832..c6934b9d998 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -68,8 +68,6 @@ func getInspectCmd(gs *globalState) *cobra.Command { inspectCmd.Flags().SortFlags = false inspectCmd.Flags().AddFlagSet(runtimeOptionFlagSet(false)) - inspectCmd.Flags().StringVarP(&gs.flags.testType, "type", "t", - gs.flags.testType, "override file `type`, \"js\" or \"archive\"") inspectCmd.Flags().BoolVar(&addExecReqs, "execution-requirements", false, diff --git a/cmd/root.go b/cmd/root.go index 9ceed327180..724efdf1e3e 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -56,7 +56,6 @@ const ( // globalFlags contains global config values that apply for all k6 sub-commands. type globalFlags struct { configFilePath string - testType string // TODO: move to RuntimeOptions, it's not trully global quiet bool noColor bool address string @@ -174,9 +173,6 @@ func getFlags(defaultFlags globalFlags, env map[string]string) globalFlags { if val, ok := env["K6_CONFIG"]; ok { result.configFilePath = val } - if val, ok := env["K6_TYPE"]; ok { - result.testType = val - } if val, ok := env["K6_LOG_OUTPUT"]; ok { result.logOutput = val } diff --git a/cmd/run.go b/cmd/run.go index e3b58cd5265..35145ae563e 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -293,7 +293,7 @@ a commandline interface for interacting with it.`, } runCmd.Flags().SortFlags = false - runCmd.Flags().AddFlagSet(runCmdFlagSet(globalState)) + runCmd.Flags().AddFlagSet(runCmdFlagSet()) return runCmd } @@ -329,23 +329,12 @@ func reportUsage(execScheduler *local.ExecutionScheduler) error { return err } -func runCmdFlagSet(globalState *globalState) *pflag.FlagSet { +func runCmdFlagSet() *pflag.FlagSet { flags := pflag.NewFlagSet("", pflag.ContinueOnError) flags.SortFlags = false flags.AddFlagSet(optionFlagSet()) flags.AddFlagSet(runtimeOptionFlagSet(true)) flags.AddFlagSet(configFlagSet()) - - // TODO: Figure out a better way to handle the CLI flags: - // - the default values are specified in this way so we don't overwrire whatever - // was specified via the environment variables - // - but we need to manually specify the DefValue, since that's the default value - // 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(&globalState.flags.testType, "type", "t", - globalState.flags.testType, "override file `type`, \"js\" or \"archive\"") - flags.Lookup("type").DefValue = "" return flags } diff --git a/cmd/runtime_options.go b/cmd/runtime_options.go index 47214875cf3..79ca302e7bd 100644 --- a/cmd/runtime_options.go +++ b/cmd/runtime_options.go @@ -47,6 +47,7 @@ base: pure goja - Golang JS VM supporting ES5.1+ extended: base + Babel with parts of ES2015 preset slower to compile in case the script uses syntax unsupported by base `) + flags.StringP("type", "t", "", "override test type, \"js\" or \"archive\"") flags.StringArrayP("env", "e", nil, "add/override environment variable with `VAR=value`") flags.Bool("no-thresholds", false, "don't run thresholds") flags.Bool("no-summary", false, "don't show the summary at the end of the test") @@ -78,6 +79,7 @@ func getRuntimeOptions(flags *pflag.FlagSet, environment map[string]string) (lib // TODO: refactor with composable helpers as a part of #883, to reduce copy-paste // TODO: get these options out of the JSON config file as well? opts := lib.RuntimeOptions{ + TestType: getNullString(flags, "type"), IncludeSystemEnvVars: getNullBool(flags, "include-system-env-vars"), CompatibilityMode: getNullString(flags, "compatibility-mode"), NoThresholds: getNullBool(flags, "no-thresholds"), @@ -86,11 +88,13 @@ func getRuntimeOptions(flags *pflag.FlagSet, environment map[string]string) (lib Env: make(map[string]string), } - if envVar, ok := environment["K6_COMPATIBILITY_MODE"]; ok { + if envVar, ok := environment["K6_TYPE"]; ok && !opts.TestType.Valid { // Only override if not explicitly set via the CLI flag - if !opts.CompatibilityMode.Valid { - opts.CompatibilityMode = null.StringFrom(envVar) - } + opts.TestType = null.StringFrom(envVar) + } + if envVar, ok := environment["K6_COMPATIBILITY_MODE"]; ok && !opts.CompatibilityMode.Valid { + // Only override if not explicitly set via the CLI flag + opts.CompatibilityMode = null.StringFrom(envVar) } if _, err := lib.ValidateCompatibilityMode(opts.CompatibilityMode.String); err != nil { // some early validation diff --git a/cmd/test_load.go b/cmd/test_load.go index 5e51fae4b25..276eb467a2f 100644 --- a/cmd/test_load.go +++ b/cmd/test_load.go @@ -89,7 +89,7 @@ func (lt *loadedTest) initializeFirstRunner(gs *globalState) error { testPath := lt.source.URL.String() logger := gs.logger.WithField("test_path", testPath) - testType := gs.flags.testType + testType := lt.runtimeOptions.TestType.String if testType == "" { logger.Debug("Detecting test type for...") testType = detectTestType(lt.source.Data) diff --git a/lib/runtime_options.go b/lib/runtime_options.go index 492684b51c1..b980c0768bc 100644 --- a/lib/runtime_options.go +++ b/lib/runtime_options.go @@ -41,6 +41,8 @@ const ( // RuntimeOptions are settings passed onto the goja JS runtime type RuntimeOptions struct { + TestType null.String `json:"-"` + // Whether to pass the actual system environment variables to the JS runtime IncludeSystemEnvVars null.Bool `json:"includeSystemEnvVars"` From 7671dc797084cf5f758295d6e42c4728a69d3f3d Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Mon, 7 Mar 2022 00:57:02 +0200 Subject: [PATCH 04/10] Simplify createOutputs() --- cmd/outputs.go | 18 +++++++----------- cmd/run.go | 2 +- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/cmd/outputs.go b/cmd/outputs.go index 107e61ec546..565a193e704 100644 --- a/cmd/outputs.go +++ b/cmd/outputs.go @@ -27,7 +27,6 @@ import ( "strings" "go.k6.io/k6/lib" - "go.k6.io/k6/loader" "go.k6.io/k6/output" "go.k6.io/k6/output/cloud" "go.k6.io/k6/output/csv" @@ -78,28 +77,25 @@ func getPossibleIDList(constrs map[string]func(output.Params) (output.Output, er return strings.Join(res, ", ") } -func createOutputs( - gs *globalState, src *loader.SourceData, conf Config, - rtOpts lib.RuntimeOptions, executionPlan []lib.ExecutionStep, -) ([]output.Output, error) { +func createOutputs(gs *globalState, test *loadedTest, executionPlan []lib.ExecutionStep) ([]output.Output, error) { outputConstructors, err := getAllOutputConstructors() if err != nil { return nil, err } baseParams := output.Params{ - ScriptPath: src.URL, + ScriptPath: test.source.URL, Logger: gs.logger, Environment: gs.envVars, StdOut: gs.stdOut, StdErr: gs.stdErr, FS: gs.fs, - ScriptOptions: conf.Options, - RuntimeOptions: rtOpts, + ScriptOptions: test.derivedConfig.Options, + RuntimeOptions: test.runtimeOptions, ExecutionPlan: executionPlan, } - result := make([]output.Output, 0, len(conf.Out)) + result := make([]output.Output, 0, len(test.derivedConfig.Out)) - for _, outputFullArg := range conf.Out { + for _, outputFullArg := range test.derivedConfig.Out { outputType, outputArg := parseOutputArgument(outputFullArg) outputConstructor, ok := outputConstructors[outputType] if !ok { @@ -112,7 +108,7 @@ func createOutputs( params := baseParams params.OutputType = outputType params.ConfigArgument = outputArg - params.JSONConfig = conf.Collectors[outputType] + params.JSONConfig = test.derivedConfig.Collectors[outputType] output, err := outputConstructor(params) if err != nil { diff --git a/cmd/run.go b/cmd/run.go index 35145ae563e..ceae89c1e8e 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -136,7 +136,7 @@ a commandline interface for interacting with it.`, // Create all outputs. executionPlan := execScheduler.GetExecutionPlan() - outputs, err := createOutputs(globalState, test.source, conf, test.runtimeOptions, executionPlan) + outputs, err := createOutputs(globalState, test, executionPlan) if err != nil { return err } From b1c270d0f3a45f3fce45ce55bd6033abc391cd64 Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Mon, 7 Mar 2022 11:27:05 +0200 Subject: [PATCH 05/10] Refactor cmd.rootCommand to support easy integration tests --- cmd/archive_test.go | 17 +----- cmd/common.go | 2 +- cmd/convert_test.go | 6 +-- cmd/integration_test.go | 115 ++++++++++++++++++++++++++++++++++++++++ cmd/root.go | 73 +++++++++++++------------ cmd/root_test.go | 18 +++++-- cmd/run.go | 2 +- cmd/run_test.go | 18 +++---- 8 files changed, 182 insertions(+), 69 deletions(-) create mode 100644 cmd/integration_test.go diff --git a/cmd/archive_test.go b/cmd/archive_test.go index 09f71abae70..9cb174c5752 100644 --- a/cmd/archive_test.go +++ b/cmd/archive_test.go @@ -6,9 +6,7 @@ import ( "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" ) @@ -51,21 +49,10 @@ func TestArchiveThresholds(t *testing.T) { testState.args = append(testState.args, "--no-thresholds") } - gotErr := newRootCommand(testState.globalState).cmd.Execute() - - assert.Equal(t, - testCase.wantErr, - gotErr != nil, - "archive command error = %v, wantErr %v", gotErr, testCase.wantErr, - ) - if testCase.wantErr { - var gotErrExt errext.HasExitCode - require.ErrorAs(t, gotErr, &gotErrExt) - assert.Equalf(t, exitcodes.InvalidConfig, gotErrExt.ExitCode(), - "status code must be %d", exitcodes.InvalidConfig, - ) + testState.expectedExitCode = int(exitcodes.InvalidConfig) } + newRootCommand(testState.globalState).execute() }) } } diff --git a/cmd/common.go b/cmd/common.go index 5b83ed53584..5d39fd950bd 100644 --- a/cmd/common.go +++ b/cmd/common.go @@ -112,7 +112,7 @@ func handleTestAbortSignals(gs *globalState, firstHandler, secondHandler func(os } // If we get a second signal, we immediately exit, so something like // https://github.com/k6io/k6/issues/971 never happens again - os.Exit(int(exitcodes.ExternalAbort)) + gs.osExit(int(exitcodes.ExternalAbort)) case <-done: return } diff --git a/cmd/convert_test.go b/cmd/convert_test.go index 5ec286c0bc0..9156f44cf81 100644 --- a/cmd/convert_test.go +++ b/cmd/convert_test.go @@ -134,7 +134,7 @@ func TestConvertCmdCorrelate(t *testing.T) { "--enable-status-code-checks=true", "--return-on-failed-check=true", "correlate.har", } - require.NoError(t, newRootCommand(testState.globalState).cmd.Execute()) + newRootCommand(testState.globalState).execute() result, err := afero.ReadFile(testState.fs, "result.js") require.NoError(t, err) @@ -166,7 +166,7 @@ func TestConvertCmdStdout(t *testing.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()) + newRootCommand(testState.globalState).execute() assert.Equal(t, testHARConvertResult, testState.stdOut.String()) } @@ -177,7 +177,7 @@ func TestConvertCmdOutputFile(t *testing.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()) + newRootCommand(testState.globalState).execute() output, err := afero.ReadFile(testState.fs, "result.js") assert.NoError(t, err) diff --git a/cmd/integration_test.go b/cmd/integration_test.go new file mode 100644 index 00000000000..846673c7986 --- /dev/null +++ b/cmd/integration_test.go @@ -0,0 +1,115 @@ +package cmd + +import ( + "bytes" + "path/filepath" + "testing" + + "github.com/sirupsen/logrus" + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.k6.io/k6/lib/testutils" +) + +const ( + noopDefaultFunc = `export default function() {};` + noopHandleSummary = ` + export function handleSummary(data) { + return {}; // silence the end of test summary + }; + ` +) + +func TestSimpleTestStdin(t *testing.T) { + t.Parallel() + + ts := newGlobalTestState(t) + ts.args = []string{"k6", "run", "-"} + ts.stdIn = bytes.NewBufferString(noopDefaultFunc) + newRootCommand(ts.globalState).execute() + + stdOut := ts.stdOut.String() + assert.Contains(t, stdOut, "default: 1 iterations for each of 1 VUs") + assert.Contains(t, stdOut, "1 complete and 0 interrupted iterations") + assert.Empty(t, ts.stdErr.Bytes()) + assert.Empty(t, ts.loggerHook.Drain()) +} + +func TestStdoutAndStderrAreEmptyWithQuietAndHandleSummary(t *testing.T) { + t.Parallel() + + ts := newGlobalTestState(t) + ts.args = []string{"k6", "--quiet", "run", "-"} + ts.stdIn = bytes.NewBufferString(noopDefaultFunc + noopHandleSummary) + newRootCommand(ts.globalState).execute() + + assert.Empty(t, ts.stdErr.Bytes()) + assert.Empty(t, ts.stdOut.Bytes()) + assert.Empty(t, ts.loggerHook.Drain()) +} + +func TestStdoutAndStderrAreEmptyWithQuietAndLogsForwarded(t *testing.T) { + t.Parallel() + + ts := newGlobalTestState(t) + + // TODO: add a test with relative path + logFilePath := filepath.Join(ts.cwd, "test.log") + + ts.args = []string{ + "k6", "--quiet", "--log-output", "file=" + logFilePath, + "--log-format", "raw", "run", "--no-summary", "-", + } + ts.stdIn = bytes.NewBufferString(`export default function() { console.log('foo'); };`) + newRootCommand(ts.globalState).execute() + + // The our test state hook still catches this message + assert.True(t, testutils.LogContains(ts.loggerHook.Drain(), logrus.InfoLevel, `foo`)) + + // But it's not shown on stderr or stdout + assert.Empty(t, ts.stdErr.Bytes()) + assert.Empty(t, ts.stdOut.Bytes()) + + // Instead it should be in the log file + logContents, err := afero.ReadFile(ts.fs, logFilePath) + require.NoError(t, err) + assert.Equal(t, "foo\n", string(logContents)) +} + +func TestWrongCliFlagIterations(t *testing.T) { + t.Parallel() + + ts := newGlobalTestState(t) + ts.args = []string{"k6", "run", "--iterations", "foo", "-"} + ts.stdIn = bytes.NewBufferString(noopDefaultFunc) + // TODO: check for exitcodes.InvalidConfig after https://github.com/loadimpact/k6/issues/883 is done... + ts.expectedExitCode = -1 + newRootCommand(ts.globalState).execute() + assert.True(t, testutils.LogContains(ts.loggerHook.Drain(), logrus.ErrorLevel, `invalid argument "foo"`)) +} + +func TestWrongEnvVarIterations(t *testing.T) { + t.Parallel() + + ts := newGlobalTestState(t) + ts.args = []string{"k6", "run", "--vus", "2", "-"} + ts.envVars = map[string]string{"K6_ITERATIONS": "4"} + ts.stdIn = bytes.NewBufferString(noopDefaultFunc) + + newRootCommand(ts.globalState).execute() + + stdOut := ts.stdOut.String() + t.Logf(stdOut) + assert.Contains(t, stdOut, "4 iterations shared among 2 VUs") + assert.Contains(t, stdOut, "4 complete and 0 interrupted iterations") + assert.Empty(t, ts.stdErr.Bytes()) + assert.Empty(t, ts.loggerHook.Drain()) +} + +// TODO: add a hell of a lot more integration tests, including some that spin up +// a test HTTP server and actually check if k6 hits it + +// TODO: also add a test that starts multiple k6 "instances", for example: +// - one with `k6 run --paused` and another with `k6 resume` +// - one with `k6 run` and another with `k6 stats` or `k6 status` diff --git a/cmd/root.go b/cmd/root.go index 724efdf1e3e..f4513eb497a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -91,11 +91,10 @@ type globalState struct { stdOut, stdErr *consoleWriter stdIn io.Reader + osExit func(int) signalNotify func(chan<- os.Signal, ...os.Signal) signalStop func(chan<- os.Signal) - // TODO: add os.Exit()? - logger *logrus.Logger fallbackLogger logrus.FieldLogger } @@ -144,6 +143,7 @@ func newGlobalState(ctx context.Context) *globalState { stdOut: stdOut, stdErr: stdErr, stdIn: os.Stdin, + osExit: os.Exit, signalNotify: signal.Notify, signalStop: signal.Stop, logger: logger, @@ -268,47 +268,52 @@ func (c *rootCommand) persistentPreRunE(cmd *cobra.Command, args []string) error return nil } -// Execute adds all child commands to the root command sets flags appropriately. -// This is called by main.main(). It only needs to happen once to the rootCmd. -func Execute() { - ctx, cancel := context.WithCancel(context.Background()) +func (c *rootCommand) execute() { + ctx, cancel := context.WithCancel(c.globalState.ctx) defer cancel() + c.globalState.ctx = ctx - globalState := newGlobalState(ctx) + err := c.cmd.Execute() + if err == nil { + cancel() + c.waitRemoteLogger() + return + } - rootCmd := newRootCommand(globalState) + exitCode := -1 + var ecerr errext.HasExitCode + if errors.As(err, &ecerr) { + exitCode = int(ecerr.ExitCode()) + } - if err := rootCmd.cmd.Execute(); err != nil { - exitCode := -1 - var ecerr errext.HasExitCode - if errors.As(err, &ecerr) { - exitCode = int(ecerr.ExitCode()) - } + errText := err.Error() + var xerr errext.Exception + if errors.As(err, &xerr) { + errText = xerr.StackTrace() + } - errText := err.Error() - var xerr errext.Exception - if errors.As(err, &xerr) { - errText = xerr.StackTrace() - } + fields := logrus.Fields{} + var herr errext.HasHint + if errors.As(err, &herr) { + fields["hint"] = herr.Hint() + } - fields := logrus.Fields{} - var herr errext.HasHint - if errors.As(err, &herr) { - fields["hint"] = herr.Hint() - } + c.globalState.logger.WithFields(fields).Error(errText) + if c.loggerIsRemote { + c.globalState.fallbackLogger.WithFields(fields).Error(errText) + cancel() + c.waitRemoteLogger() + } - globalState.logger.WithFields(fields).Error(errText) - if rootCmd.loggerIsRemote { - globalState.fallbackLogger.WithFields(fields).Error(errText) - cancel() - rootCmd.waitRemoteLogger() - } + c.globalState.osExit(exitCode) +} - os.Exit(exitCode) //nolint:gocritic - } +// Execute adds all child commands to the root command sets flags appropriately. +// This is called by main.main(). It only needs to happen once to the rootCmd. +func Execute() { + gs := newGlobalState(context.Background()) - cancel() - rootCmd.waitRemoteLogger() + newRootCommand(gs).execute() } func (c *rootCommand) waitRemoteLogger() { diff --git a/cmd/root_test.go b/cmd/root_test.go index c620a8c5a1d..2c3a7f25659 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -23,6 +23,8 @@ type globalTestState struct { loggerHook *testutils.SimpleLogrusHook cwd string + + expectedExitCode int } func newGlobalTestState(t *testing.T) *globalTestState { @@ -50,8 +52,19 @@ func newGlobalTestState(t *testing.T) *globalTestState { stdErr: new(bytes.Buffer), } + defaultOsExitHandle := func(exitCode int) { + require.Equal(t, ts.expectedExitCode, exitCode) + cancel() + } + outMutex := &sync.Mutex{} defaultFlags := getDefaultFlags(".config") + + // Set an empty REST API address by default so that `k6 run` dosen't try to + // bind to it, which will result in parallel integration tests trying to use + // the same port and a warning message in every one. + defaultFlags.address = "" + ts.globalState = &globalState{ ctx: ctx, fs: fs, @@ -64,6 +77,7 @@ func newGlobalTestState(t *testing.T) *globalTestState { stdOut: &consoleWriter{nil, ts.stdOut, false, outMutex, nil}, stdErr: &consoleWriter{nil, ts.stdErr, false, outMutex, nil}, stdIn: new(bytes.Buffer), + osExit: defaultOsExitHandle, signalNotify: signal.Notify, signalStop: signal.Stop, logger: logger, @@ -82,9 +96,7 @@ func TestDeprecatedOptionWarning(t *testing.T) { export default function() { console.log('bar'); }; `)) - root := newRootCommand(ts.globalState) - - require.NoError(t, root.cmd.Execute()) + newRootCommand(ts.globalState).execute() logMsgs := ts.loggerHook.Drain() assert.True(t, testutils.LogContains(logMsgs, logrus.InfoLevel, "foo")) diff --git a/cmd/run.go b/cmd/run.go index ceae89c1e8e..029e900c83f 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -160,7 +160,7 @@ a commandline interface for interacting with it.`, // 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") - os.Exit(int(exitcodes.CannotStartRESTAPI)) + globalState.osExit(int(exitcodes.CannotStartRESTAPI)) } else { logger.WithError(aerr).Warn("Error from API server") } diff --git a/cmd/run_test.go b/cmd/run_test.go index ba1af0cfc7f..1648fcaf9f1 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -197,23 +197,17 @@ func TestRunScriptErrorsAndAbort(t *testing.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() + testState.expectedExitCode = int(tc.expExitCode) + newRootCommand(testState.globalState).execute() - if tc.expErr != "" { - require.Error(t, err) - assert.Contains(t, err.Error(), tc.expErr) - } else { - require.NoError(t, err) - } + logs := testState.loggerHook.Drain() - 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.expErr != "" { + assert.True(t, testutils.LogContains(logs, logrus.ErrorLevel, tc.expErr)) } if tc.expLogOutput != "" { - assert.True(t, testutils.LogContains(testState.loggerHook.Drain(), logrus.InfoLevel, tc.expLogOutput)) + assert.True(t, testutils.LogContains(logs, logrus.InfoLevel, tc.expLogOutput)) } }) } From 0581cc4a7c681d06f23b628fb97b033005b83ad8 Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Mon, 7 Mar 2022 14:56:14 +0200 Subject: [PATCH 06/10] Fix --log-output=file support for relative paths and add a test --- cmd/integration_test.go | 29 +++++++++++++++++++++++++++-- cmd/root.go | 5 ++++- log/file.go | 26 +++++++++++++++++--------- log/file_test.go | 6 +++++- 4 files changed, 53 insertions(+), 13 deletions(-) diff --git a/cmd/integration_test.go b/cmd/integration_test.go index 846673c7986..ae60195d6ce 100644 --- a/cmd/integration_test.go +++ b/cmd/integration_test.go @@ -14,6 +14,7 @@ import ( const ( noopDefaultFunc = `export default function() {};` + fooLogDefaultFunc = `export default function() { console.log('foo'); };` noopHandleSummary = ` export function handleSummary(data) { return {}; // silence the end of test summary @@ -61,10 +62,10 @@ func TestStdoutAndStderrAreEmptyWithQuietAndLogsForwarded(t *testing.T) { "k6", "--quiet", "--log-output", "file=" + logFilePath, "--log-format", "raw", "run", "--no-summary", "-", } - ts.stdIn = bytes.NewBufferString(`export default function() { console.log('foo'); };`) + ts.stdIn = bytes.NewBufferString(fooLogDefaultFunc) newRootCommand(ts.globalState).execute() - // The our test state hook still catches this message + // The test state hook still catches this message assert.True(t, testutils.LogContains(ts.loggerHook.Drain(), logrus.InfoLevel, `foo`)) // But it's not shown on stderr or stdout @@ -77,6 +78,30 @@ func TestStdoutAndStderrAreEmptyWithQuietAndLogsForwarded(t *testing.T) { assert.Equal(t, "foo\n", string(logContents)) } +func TestRelativeLogPathWithSetupAndTeardown(t *testing.T) { + t.Parallel() + + ts := newGlobalTestState(t) + + ts.args = []string{"k6", "--log-output", "file=test.log", "--log-format", "raw", "run", "-i", "2", "-"} + ts.stdIn = bytes.NewBufferString(fooLogDefaultFunc + ` + export function setup() { console.log('bar'); }; + export function teardown() { console.log('baz'); }; + `) + newRootCommand(ts.globalState).execute() + + // The test state hook still catches these messages + logEntries := ts.loggerHook.Drain() + assert.True(t, testutils.LogContains(logEntries, logrus.InfoLevel, `foo`)) + assert.True(t, testutils.LogContains(logEntries, logrus.InfoLevel, `bar`)) + assert.True(t, testutils.LogContains(logEntries, logrus.InfoLevel, `baz`)) + + // And check that the log file also contains everything + logContents, err := afero.ReadFile(ts.fs, filepath.Join(ts.cwd, "test.log")) + require.NoError(t, err) + assert.Equal(t, "bar\nfoo\nfoo\nbaz\n", string(logContents)) +} + func TestWrongCliFlagIterations(t *testing.T) { t.Parallel() diff --git a/cmd/root.go b/cmd/root.go index f4513eb497a..13dd7a9b801 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -410,7 +410,10 @@ func (c *rootCommand) setupLoggers() (<-chan struct{}, error) { case strings.HasPrefix(line, "file"): ch = make(chan struct{}) // TODO: refactor, get it from the constructor - hook, err := log.FileHookFromConfigLine(c.globalState.ctx, c.globalState.fs, c.globalState.fallbackLogger, line, ch) + hook, err := log.FileHookFromConfigLine( + c.globalState.ctx, c.globalState.fs, c.globalState.getwd, + c.globalState.fallbackLogger, line, ch, + ) if err != nil { return nil, err } diff --git a/log/file.go b/log/file.go index 2901c3f367c..0cd87376c04 100644 --- a/log/file.go +++ b/log/file.go @@ -51,10 +51,9 @@ type fileHook struct { // FileHookFromConfigLine returns new fileHook hook. func FileHookFromConfigLine( - ctx context.Context, fs afero.Fs, fallbackLogger logrus.FieldLogger, line string, done chan struct{}, + ctx context.Context, fs afero.Fs, getCwd func() (string, error), + 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, @@ -71,7 +70,7 @@ func FileHookFromConfigLine( return nil, err } - if err := hook.openFile(); err != nil { + if err := hook.openFile(getCwd); err != nil { return nil, err } @@ -107,14 +106,23 @@ func (h *fileHook) parseArgs(line string) error { } // openFile opens logfile and initializes writers. -func (h *fileHook) openFile() error { - 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)) +func (h *fileHook) openFile(getCwd func() (string, error)) error { + path := h.path + if !filepath.IsAbs(path) { + cwd, err := getCwd() + if err != nil { + return fmt.Errorf("'%s' is a relative path but could not determine CWD: %w", path, err) + } + path = filepath.Join(cwd, path) + } + + if _, err := h.fs.Stat(filepath.Dir(path)); os.IsNotExist(err) { + return fmt.Errorf("provided directory '%s' does not exist", filepath.Dir(path)) } - file, err := h.fs.OpenFile(h.path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o600) + file, err := h.fs.OpenFile(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) + return fmt.Errorf("failed to open logfile %s: %w", path, err) } h.w = file diff --git a/log/file_test.go b/log/file_test.go index ca4417f2bc5..b959aa2d962 100644 --- a/log/file_test.go +++ b/log/file_test.go @@ -110,8 +110,12 @@ func TestFileHookFromConfigLine(t *testing.T) { t.Run(test.line, func(t *testing.T) { t.Parallel() + getCwd := func() (string, error) { + return "/", nil + } + res, err := FileHookFromConfigLine( - context.Background(), afero.NewMemMapFs(), logrus.New(), test.line, make(chan struct{}), + context.Background(), afero.NewMemMapFs(), getCwd, logrus.New(), test.line, make(chan struct{}), ) if test.err { From 3b3e3860249aec608ebc3666e1f10ceb1fd8594d Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Mon, 7 Mar 2022 17:02:25 +0200 Subject: [PATCH 07/10] Rename and document better the cmd/inspect.go internals --- cmd/inspect.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/cmd/inspect.go b/cmd/inspect.go index c6934b9d998..e154665ad15 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -47,13 +47,14 @@ func getInspectCmd(gs *globalState) *cobra.Command { // At the moment, `k6 inspect` output can take 2 forms: standard // (equal to the lib.Options struct) and extended, with additional // fields with execution requirements. - inspectOutput := interface{}(test.initRunner.GetOptions()) - + var inspectOutput interface{} if addExecReqs { - inspectOutput, err = addExecRequirements(gs, cmd, test) + inspectOutput, err = inspectOutputWithExecRequirements(gs, cmd, test) if err != nil { return err } + } else { + inspectOutput = test.initRunner.GetOptions() } data, err := json.MarshalIndent(inspectOutput, "", " ") @@ -76,7 +77,9 @@ func getInspectCmd(gs *globalState) *cobra.Command { return inspectCmd } -func addExecRequirements(gs *globalState, cmd *cobra.Command, test *loadedTest) (interface{}, error) { +// If --execution-requirements is enabled, this will consolidate the config, +// derive the value of `scenarios` and calculate the max test duration and VUs. +func inspectOutputWithExecRequirements(gs *globalState, cmd *cobra.Command, test *loadedTest) (interface{}, error) { // we don't actually support CLI flags here, so we pass nil as the getter if err := test.consolidateDeriveAndValidateConfig(gs, cmd, nil); err != nil { return nil, err From 85c5169f89e01e285f1660199042d21683183ed2 Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Mon, 7 Mar 2022 18:37:22 +0200 Subject: [PATCH 08/10] Remove js.Bundle.IsExecutable() because it's not needed anymore --- js/bundle.go | 7 ------- js/runner.go | 5 ++--- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/js/bundle.go b/js/bundle.go index b13adc26639..d80ec7a2b27 100644 --- a/js/bundle.go +++ b/js/bundle.go @@ -296,13 +296,6 @@ func (b *Bundle) Instantiate( return bi, instErr } -// IsExecutable returns whether the given name is an exported and -// executable function in the script. -func (b *Bundle) IsExecutable(name string) bool { - _, exists := b.exports[name] - return exists -} - // Instantiates the bundle into an existing runtime. Not public because it also messes with a bunch // of other things, will potentially thrash data and makes a mess in it if the operation fails. func (b *Bundle) instantiate(logger logrus.FieldLogger, rt *goja.Runtime, init *InitContext, vuID uint64) error { diff --git a/js/runner.go b/js/runner.go index 39bbed12be3..1b6c5c948e1 100644 --- a/js/runner.go +++ b/js/runner.go @@ -364,10 +364,9 @@ func (r *Runner) GetOptions() lib.Options { // IsExecutable returns whether the given name is an exported and // executable function in the script. -// -// TODO: completely remove this? func (r *Runner) IsExecutable(name string) bool { - return r.Bundle.IsExecutable(name) + _, exists := r.Bundle.exports[name] + return exists } // HandleSummary calls the specified summary callback, if supplied. From 1455a62e6e728ceb965cb574dab3ec41b5271d8e Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Tue, 8 Mar 2022 15:05:18 +0200 Subject: [PATCH 09/10] Move some output init calls away from the Engine --- cmd/outputs.go | 13 +++++++++++-- core/engine.go | 8 -------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/cmd/outputs.go b/cmd/outputs.go index 565a193e704..08893380f4f 100644 --- a/cmd/outputs.go +++ b/cmd/outputs.go @@ -110,11 +110,20 @@ func createOutputs(gs *globalState, test *loadedTest, executionPlan []lib.Execut params.ConfigArgument = outputArg params.JSONConfig = test.derivedConfig.Collectors[outputType] - output, err := outputConstructor(params) + out, err := outputConstructor(params) if err != nil { return nil, fmt.Errorf("could not create the '%s' output: %w", outputType, err) } - result = append(result, output) + + if thresholdOut, ok := out.(output.WithThresholds); ok { + thresholdOut.SetThresholds(test.derivedConfig.Thresholds) + } + + if builtinMetricOut, ok := out.(output.WithBuiltinMetrics); ok { + builtinMetricOut.SetBuiltinMetrics(test.builtInMetrics) + } + + result = append(result, out) } return result, nil diff --git a/core/engine.go b/core/engine.go index 599421e187c..805f5527ce7 100644 --- a/core/engine.go +++ b/core/engine.go @@ -139,10 +139,6 @@ func NewEngine( func (e *Engine) StartOutputs() error { e.logger.Debugf("Starting %d outputs...", len(e.outputs)) for i, out := range e.outputs { - if thresholdOut, ok := out.(output.WithThresholds); ok { - thresholdOut.SetThresholds(e.thresholds) - } - if stopOut, ok := out.(output.WithTestRunStop); ok { stopOut.SetTestRunStopCallback( func(err error) { @@ -151,10 +147,6 @@ func (e *Engine) StartOutputs() error { }) } - if builtinMetricOut, ok := out.(output.WithBuiltinMetrics); ok { - builtinMetricOut.SetBuiltinMetrics(e.builtinMetrics) - } - if err := out.Start(); err != nil { e.stopOutputs(i) return err From 6ffdb958f5e13bdc77fa7ef317af29c7be9b55a0 Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Wed, 16 Mar 2022 18:03:05 +0200 Subject: [PATCH 10/10] Improve some code identifier names and comments --- cmd/archive.go | 2 +- cmd/cloud.go | 6 +++--- cmd/inspect.go | 2 +- cmd/run.go | 2 +- cmd/runtime_options_test.go | 4 ++-- cmd/test_load.go | 25 +++++++++++++++---------- 6 files changed, 23 insertions(+), 18 deletions(-) diff --git a/cmd/archive.go b/cmd/archive.go index bd57e53468a..8cf09653016 100644 --- a/cmd/archive.go +++ b/cmd/archive.go @@ -42,7 +42,7 @@ 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 { - test, err := loadTest(gs, cmd, args, getPartialConfig) + test, err := loadAndConfigureTest(gs, cmd, args, getPartialConfig) if err != nil { return err } diff --git a/cmd/cloud.go b/cmd/cloud.go index b51df6df3a2..4c6e28ecd9a 100644 --- a/cmd/cloud.go +++ b/cmd/cloud.go @@ -95,7 +95,7 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth ) printBar(globalState, progressBar) - test, err := loadTest(globalState, cmd, args, getPartialConfig) + test, err := loadAndConfigureTest(globalState, cmd, args, getPartialConfig) if err != nil { return err } @@ -166,7 +166,7 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth name := cloudConfig.Name.String if !cloudConfig.Name.Valid || cloudConfig.Name.String == "" { - name = filepath.Base(test.testPath) + name = filepath.Base(test.sourceRootPath) } globalCtx, globalCancel := context.WithCancel(globalState.ctx) @@ -216,7 +216,7 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth testURL := cloudapi.URLForResults(refID, cloudConfig) executionPlan := test.derivedConfig.Scenarios.GetFullExecutionRequirements(et) printExecutionDescription( - globalState, "cloud", test.testPath, testURL, test.derivedConfig, et, executionPlan, nil, + globalState, "cloud", test.sourceRootPath, testURL, test.derivedConfig, et, executionPlan, nil, ) modifyAndPrintBar( diff --git a/cmd/inspect.go b/cmd/inspect.go index e154665ad15..4a12f944762 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -39,7 +39,7 @@ func getInspectCmd(gs *globalState) *cobra.Command { Long: `Inspect a script or archive.`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - test, err := loadTest(gs, cmd, args, nil) + test, err := loadAndConfigureTest(gs, cmd, args, nil) if err != nil { return err } diff --git a/cmd/run.go b/cmd/run.go index 029e900c83f..59a87a5d9b6 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -80,7 +80,7 @@ a commandline interface for interacting with it.`, RunE: func(cmd *cobra.Command, args []string) error { printBanner(globalState) - test, err := loadTest(globalState, cmd, args, getConfig) + test, err := loadAndConfigureTest(globalState, cmd, args, getConfig) if err != nil { return err } diff --git a/cmd/runtime_options_test.go b/cmd/runtime_options_test.go index 65107db7d25..823c4e2330f 100644 --- a/cmd/runtime_options_test.go +++ b/cmd/runtime_options_test.go @@ -81,7 +81,7 @@ func testRuntimeOptionsCase(t *testing.T, tc runtimeOptionsTestCase) { ts := newGlobalTestState(t) // TODO: move upwards, make this into an almost full integration test registry := metrics.NewRegistry() test := &loadedTest{ - testPath: "script.js", + sourceRootPath: "script.js", source: &loader.SourceData{Data: jsCode.Bytes(), URL: &url.URL{Path: "/script.js", Scheme: "file"}}, fileSystems: map[string]afero.Fs{"file": fs}, runtimeOptions: rtOpts, @@ -97,7 +97,7 @@ func testRuntimeOptionsCase(t *testing.T, tc runtimeOptionsTestCase) { getRunnerErr := func(rtOpts lib.RuntimeOptions) *loadedTest { return &loadedTest{ - testPath: "script.tar", + sourceRootPath: "script.tar", source: &loader.SourceData{Data: archiveBuf.Bytes(), URL: &url.URL{Path: "/script.tar", Scheme: "file"}}, fileSystems: map[string]afero.Fs{"file": fs}, runtimeOptions: rtOpts, diff --git a/cmd/test_load.go b/cmd/test_load.go index 276eb467a2f..29d2952dffe 100644 --- a/cmd/test_load.go +++ b/cmd/test_load.go @@ -21,8 +21,10 @@ const ( testTypeArchive = "archive" ) +// loadedTest contains all of data, details and dependencies of a fully-loaded +// and configured k6 test. type loadedTest struct { - testPath string // contains the raw string the user supplied + sourceRootPath string // contains the raw string the user supplied source *loader.SourceData fileSystems map[string]afero.Fs runtimeOptions lib.RuntimeOptions @@ -30,13 +32,13 @@ type loadedTest struct { builtInMetrics *metrics.BuiltinMetrics initRunner lib.Runner // TODO: rename to something more appropriate - // Only set if cliConfigGetter is supplied to loadTest() or if + // Only set if cliConfigGetter is supplied to loadAndConfigureTest() or if // consolidateDeriveAndValidateConfig() is manually called. consolidatedConfig Config derivedConfig Config } -func loadTest( +func loadAndConfigureTest( gs *globalState, cmd *cobra.Command, args []string, // supply this if you want the test config consolidated and validated cliConfigGetter func(flags *pflag.FlagSet) (Config, error), // TODO: obviate @@ -45,14 +47,17 @@ func loadTest( return nil, fmt.Errorf("k6 needs at least one argument to load the test") } - testPath := args[0] - gs.logger.Debugf("Resolving and reading test '%s'...", testPath) - src, fileSystems, err := readSource(gs, testPath) + sourceRootPath := args[0] + gs.logger.Debugf("Resolving and reading test '%s'...", sourceRootPath) + src, fileSystems, err := readSource(gs, sourceRootPath) if err != nil { return nil, err } resolvedPath := src.URL.String() - gs.logger.Debugf("'%s' resolved to '%s' and successfully loaded %d bytes!", testPath, resolvedPath, len(src.Data)) + gs.logger.Debugf( + "'%s' resolved to '%s' and successfully loaded %d bytes!", + sourceRootPath, resolvedPath, len(src.Data), + ) gs.logger.Debugf("Gathering k6 runtime options...") runtimeOptions, err := getRuntimeOptions(cmd.Flags(), gs.envVars) @@ -62,7 +67,7 @@ func loadTest( registry := metrics.NewRegistry() test := &loadedTest{ - testPath: testPath, + sourceRootPath: sourceRootPath, source: src, fileSystems: fileSystems, runtimeOptions: runtimeOptions, @@ -70,9 +75,9 @@ func loadTest( builtInMetrics: metrics.RegisterBuiltinMetrics(registry), } - gs.logger.Debugf("Initializing k6 runner for '%s' (%s)...", testPath, resolvedPath) + gs.logger.Debugf("Initializing k6 runner for '%s' (%s)...", sourceRootPath, resolvedPath) if err := test.initializeFirstRunner(gs); err != nil { - return nil, fmt.Errorf("could not initialize '%s': %w", testPath, err) + return nil, fmt.Errorf("could not initialize '%s': %w", sourceRootPath, err) } gs.logger.Debug("Runner successfully initialized!")